Skip to content

N
nexpie-grafana-theme
Commit 9ba8114b by Hugo Häggmark Committed by GitHub
Options

Variables: New Variables are stored immediately (#29178) 

* Refactor: Adds getNextAvailableId

* Refactor: Hides DependencyGraph button if there are no dependencies

* Refactor: Changes the new button

* Refactor: Removes usages of NEW_VARIABLE_ID

* Refactor: Reverts the new button
parent cf6512ab
Showing with 88 additions and 157 deletions
public/app/features/variables/editor/VariableEditorContainer.tsx
......@@ -2,7 +2,7 @@ import React, { MouseEvent, PureComponent } from 'react';
import { Icon } from '@grafana/ui';
import { selectors } from '@grafana/e2e-selectors';
import { NEW_VARIABLE_ID, toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types';
import { toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types';
import { StoreState } from '../../../types';
import { VariableEditorEditor } from './VariableEditorEditor';
import { MapDispatchToProps, MapStateToProps } from 'react-redux';
......@@ -78,16 +78,7 @@ class VariableEditorContainerUnconnected extends PureComponent<Props> {
>
Variables
</a>
{this.props.idInEditor === NEW_VARIABLE_ID && (
<span>
<Icon
name="angle-right"
aria-label={selectors.pages.Dashboard.Settings.Variables.Edit.General.modeLabelNew}
/>
New
</span>
)}
{this.props.idInEditor && this.props.idInEditor !== NEW_VARIABLE_ID && (
{this.props.idInEditor && (
<span>
<Icon
name="angle-right"
......
public/app/features/variables/editor/VariableEditorEditor.tsx
......@@ -5,11 +5,11 @@ import { Icon, InlineFormLabel } from '@grafana/ui';
import { selectors } from '@grafana/e2e-selectors';
import { variableAdapters } from '../adapters';
import { NEW_VARIABLE_ID, toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types';
import { toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types';
import { VariableHide, VariableModel } from '../types';
import { appEvents } from '../../../core/core';
import { VariableValuesPreview } from './VariableValuesPreview';
import { changeVariableName, onEditorAdd, onEditorUpdate, variableEditorMount, variableEditorUnMount } from './actions';
import { changeVariableName, onEditorUpdate, variableEditorMount, variableEditorUnMount } from './actions';
import { MapDispatchToProps, MapStateToProps } from 'react-redux';
import { StoreState } from '../../../types';
import { VariableEditorState } from './reducer';
......@@ -18,6 +18,7 @@ import { connectWithStore } from '../../../core/utils/connectWithReduxStore';
import { OnPropChangeArguments } from './types';
import { changeVariableProp, changeVariableType } from '../state/sharedReducer';
import { updateOptions } from '../state/actions';
import { getVariableTypes } from '../utils';
export interface OwnProps {
identifier: VariableIdentifier;
......@@ -34,7 +35,6 @@ interface DispatchProps {
changeVariableName: typeof changeVariableName;
changeVariableProp: typeof changeVariableProp;
onEditorUpdate: typeof onEditorUpdate;
onEditorAdd: typeof onEditorAdd;
changeVariableType: typeof changeVariableType;
updateOptions: typeof updateOptions;
}
......@@ -100,13 +100,7 @@ export class VariableEditorEditorUnConnected extends PureComponent<Props> {
return;
}
if (this.props.variable.id !== NEW_VARIABLE_ID) {
await this.props.onEditorUpdate(this.props.identifier);
}
if (this.props.variable.id === NEW_VARIABLE_ID) {
await this.props.onEditorAdd(this.props.identifier);
}
await this.props.onEditorUpdate(this.props.identifier);
};
render() {
......@@ -115,7 +109,6 @@ export class VariableEditorEditorUnConnected extends PureComponent<Props> {
if (!EditorToRender) {
return null;
}
const newVariable = this.props.variable.id && this.props.variable.id === NEW_VARIABLE_ID;
const loading = variable.state === LoadingState.Loading;
return (
......@@ -148,8 +141,8 @@ export class VariableEditorEditorUnConnected extends PureComponent<Props> {
onChange={this.onTypeChange}
aria-label={selectors.pages.Dashboard.Settings.Variables.Edit.General.generalTypeSelect}
>
{variableAdapters.list().map(({ id, name }) => (
<option key={id} label={name} value={id}>
{getVariableTypes().map(({ label, value }) => (
<option key={value} label={label} value={value}>
{name}
</option>
))}
......@@ -211,7 +204,7 @@ export class VariableEditorEditorUnConnected extends PureComponent<Props> {
aria-label={selectors.pages.Dashboard.Settings.Variables.Edit.General.submitButton}
disabled={loading}
>
{newVariable ? 'Add' : 'Update'}
Update
{loading ? <Icon className="spin-clockwise" name="sync" size="sm" style={{ marginLeft: '2px' }} /> : null}
</button>
</div>
......@@ -232,7 +225,6 @@ const mapDispatchToProps: MapDispatchToProps<DispatchProps, OwnProps> = {
changeVariableName,
changeVariableProp,
onEditorUpdate,
onEditorAdd,
changeVariableType,
updateOptions,
};
......
public/app/features/variables/editor/actions.test.ts 0 → 100644
import { constantBuilder, customBuilder } from '../shared/testing/builders';
import { getNextAvailableId } from './actions';
describe('getNextAvailableId', () => {
describe('when called with a custom type and there is already 2 variables', () => {
it('then the correct id should be created', () => {
const custom1 = customBuilder()
.withId('custom0')
.withName('custom0')
.build();
const constant1 = constantBuilder()
.withId('custom1')
.withName('custom1')
.build();
const variables = [custom1, constant1];
const type = 'custom';
const result = getNextAvailableId(type, variables);
expect(result).toEqual('custom2');
});
});
});
public/app/features/variables/editor/actions.ts
......@@ -9,17 +9,12 @@ import {
variableEditorUnMounted,
} from './reducer';
import { variableAdapters } from '../adapters';
import {
AddVariable,
NEW_VARIABLE_ID,
toVariableIdentifier,
toVariablePayload,
VariableIdentifier,
} from '../state/types';
import { AddVariable, toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types';
import cloneDeep from 'lodash/cloneDeep';
import { VariableType } from '@grafana/data';
import { addVariable, removeVariable, storeNewVariable } from '../state/sharedReducer';
import { addVariable, removeVariable } from '../state/sharedReducer';
import { updateOptions } from '../state/actions';
import { VariableModel } from '../types';
export const variableEditorMount = (identifier: VariableIdentifier): ThunkResult<void> => {
return async dispatch => {
......@@ -30,9 +25,6 @@ export const variableEditorMount = (identifier: VariableIdentifier): ThunkResult
export const variableEditorUnMount = (identifier: VariableIdentifier): ThunkResult<void> => {
return async (dispatch, getState) => {
dispatch(variableEditorUnMounted(toVariablePayload(identifier)));
if (getState().templating.variables[NEW_VARIABLE_ID]) {
dispatch(removeVariable(toVariablePayload({ type: identifier.type, id: NEW_VARIABLE_ID }, { reIndex: false })));
}
};
};
......@@ -43,17 +35,6 @@ export const onEditorUpdate = (identifier: VariableIdentifier): ThunkResult<void
};
};
export const onEditorAdd = (identifier: VariableIdentifier): ThunkResult<void> => {
return async (dispatch, getState) => {
const newVariableInState = getVariable(NEW_VARIABLE_ID, getState());
const id = newVariableInState.name;
dispatch(storeNewVariable(toVariablePayload({ type: identifier.type, id })));
await dispatch(updateOptions(identifier));
dispatch(switchToListMode());
dispatch(removeVariable(toVariablePayload({ type: identifier.type, id: NEW_VARIABLE_ID }, { reIndex: false })));
};
};
export const changeVariableName = (identifier: VariableIdentifier, newName: string): ThunkResult<void> => {
return (dispatch, getState) => {
let errorText = null;
......@@ -77,18 +58,10 @@ export const changeVariableName = (identifier: VariableIdentifier, newName: stri
return;
}
const thunkToCall = identifier.id === NEW_VARIABLE_ID ? completeChangeNewVariableName : completeChangeVariableName;
dispatch(thunkToCall(identifier, newName));
dispatch(completeChangeVariableName(identifier, newName));
};
};
export const completeChangeNewVariableName = (
identifier: VariableIdentifier,
newName: string
): ThunkResult<void> => dispatch => {
dispatch(changeVariableNameSucceeded(toVariablePayload(identifier, { newName })));
};
export const completeChangeVariableName = (identifier: VariableIdentifier, newName: string): ThunkResult<void> => (
dispatch,
getState
......@@ -109,13 +82,14 @@ export const completeChangeVariableName = (identifier: VariableIdentifier, newNa
dispatch(removeVariable(toVariablePayload(identifier, { reIndex: false })));
};
export const switchToNewMode = (): ThunkResult<void> => (dispatch, getState) => {
const type: VariableType = 'query';
const id = NEW_VARIABLE_ID;
export const switchToNewMode = (type: VariableType = 'query'): ThunkResult<void> => (dispatch, getState) => {
const id = getNextAvailableId(type, getVariables(getState()));
const identifier = { type, id };
const global = false;
const model = cloneDeep(variableAdapters.get(type).initialState);
const index = getNewVariabelIndex(getState());
const identifier = { type, id };
const model = cloneDeep(variableAdapters.get(type).initialState);
model.id = id;
model.name = id;
dispatch(
addVariable(
toVariablePayload<AddVariable>(identifier, { global, model, index })
......@@ -131,3 +105,14 @@ export const switchToEditMode = (identifier: VariableIdentifier): ThunkResult<vo
export const switchToListMode = (): ThunkResult<void> => dispatch => {
dispatch(clearIdInEditor());
};
export function getNextAvailableId(type: VariableType, variables: VariableModel[]): string {
let counter = 0;
let nextId = `${type}${counter}`;
while (variables.find(variable => variable.id === nextId)) {
nextId = `${type}${++counter}`;
}
return nextId;
}
public/app/features/variables/inspect/VariablesDependenciesButton.tsx
......@@ -21,6 +21,10 @@ export const UnProvidedVariablesDependenciesButton: FC<Props> = ({ variables })
const nodes = useMemo(() => createDependencyNodes(variables), [variables]);
const edges = useMemo(() => createDependencyEdges(variables), [variables]);
if (!edges.length) {
return null;
}
return (
<NetworkGraphModal
show={false}
......
public/app/features/variables/query/QueryVariableEditor.tsx
......@@ -62,7 +62,7 @@ export class QueryVariableEditorUnConnected extends PureComponent<Props, State>
}
getSelectedDataSourceValue = (): string => {
if (!this.props.editor.extended?.dataSources.length) {
if (!this.props.editor.extended?.dataSources?.length) {
return '';
}
const foundItem = this.props.editor.extended?.dataSources.find(ds => ds.value === this.props.variable.datasource);
......@@ -201,7 +201,7 @@ export class QueryVariableEditorUnConnected extends PureComponent<Props, State>
selectors.pages.Dashboard.Settings.Variables.Edit.QueryVariable.queryOptionsDataSourceSelect
}
>
{this.props.editor.extended?.dataSources.length &&
{this.props.editor.extended?.dataSources?.length &&
this.props.editor.extended?.dataSources.map(ds => (
<option key={ds.value ?? ''} value={ds.value ?? ''} label={ds.name}>
{ds.name}
......
public/app/features/variables/state/actions.test.ts
......@@ -428,7 +428,18 @@ describe('shared actions', () => {
)
.whenActionIsDispatched(changeVariableName(toVariableIdentifier(constant), 'constant1'), true)
.thenDispatchedActionsShouldEqual(
changeVariableNameSucceeded({ type: 'constant', id: NEW_VARIABLE_ID, data: { newName: 'constant1' } })
addVariable({
type: 'constant',
id: 'constant1',
data: {
global: false,
index: 1,
model: { ...constant, name: 'constant1', id: 'constant1', global: false, index: 1 },
},
}),
changeVariableNameSucceeded({ type: 'constant', id: 'constant1', data: { newName: 'constant1' } }),
setIdInEditor({ id: 'constant1' }),
removeVariable({ type: 'constant', id: NEW_VARIABLE_ID, data: { reIndex: false } })
);
});
});
......
public/app/features/variables/state/selectors.ts
import { StoreState } from '../../../types';
import { VariableModel } from '../types';
import { getState } from '../../../store/store';
import { NEW_VARIABLE_ID } from './types';
import memoizeOne from 'memoize-one';
export const getVariable = <T extends VariableModel = VariableModel>(
......@@ -29,15 +28,9 @@ export const getVariableWithName = (name: string, state: StoreState = getState()
return getVariable(name, state, false);
};
export const getVariables = (state: StoreState = getState(), includeNewVariable = false): VariableModel[] => {
export const getVariables = (state: StoreState = getState()): VariableModel[] => {
const filter = (variable: VariableModel) => {
if (variable.type === 'system') {
return false;
}
if (includeNewVariable) {
return true;
}
return variable.id !== NEW_VARIABLE_ID;
return variable.type !== 'system';
};
return getFilteredVariables(filter, state);
......@@ -48,7 +41,7 @@ export const getSubMenuVariables = memoizeOne((variables: Record<string, Variabl
});
export const getEditorVariables = (state: StoreState): VariableModel[] => {
return getVariables(state, true);
return getVariables(state);
};
export type GetVariables = typeof getVariables;
......
public/app/features/variables/state/sharedReducer.test.ts
......@@ -11,14 +11,13 @@ import {
removeVariable,
setCurrentVariableValue,
sharedReducer,
storeNewVariable,
variableStateCompleted,
variableStateFailed,
variableStateFetching,
variableStateNotStarted,
} from './sharedReducer';
import { QueryVariableModel, VariableHide } from '../types';
import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, NEW_VARIABLE_ID, toVariablePayload } from './types';
import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, toVariablePayload } from './types';
import { variableAdapters } from '../adapters';
import { createQueryVariableAdapter } from '../query/adapter';
import { initialQueryVariableModelState } from '../query/reducer';
......@@ -228,73 +227,6 @@ describe('sharedReducer', () => {
});
});
describe('when storeNewVariable is dispatched', () => {
it('then state should be correct', () => {
const initialState: VariablesState = getVariableState(3, -1, true);
const payload = toVariablePayload({ id: '11', type: 'query' });
reducerTester<VariablesState>()
.givenReducer(sharedReducer, initialState)
.whenActionIsDispatched(storeNewVariable(payload))
.thenStateShouldEqual({
'0': {
id: '0',
type: 'query',
name: 'Name-0',
hide: VariableHide.dontHide,
index: 0,
label: 'Label-0',
skipUrlSync: false,
global: false,
state: LoadingState.NotStarted,
error: null,
},
'1': {
id: '1',
type: 'query',
name: 'Name-1',
hide: VariableHide.dontHide,
index: 1,
label: 'Label-1',
skipUrlSync: false,
global: false,
state: LoadingState.NotStarted,
error: null,
},
'2': {
id: '2',
type: 'query',
name: 'Name-2',
hide: VariableHide.dontHide,
index: 2,
label: 'Label-2',
skipUrlSync: false,
global: false,
state: LoadingState.NotStarted,
error: null,
},
[NEW_VARIABLE_ID]: {
id: NEW_VARIABLE_ID,
type: 'query',
name: `Name-${NEW_VARIABLE_ID}`,
hide: VariableHide.dontHide,
index: 3,
label: `Label-${NEW_VARIABLE_ID}`,
skipUrlSync: false,
global: false,
state: LoadingState.NotStarted,
error: null,
},
[11]: {
...initialQueryVariableModelState,
id: '11',
name: `Name-${NEW_VARIABLE_ID}`,
index: 3,
label: `Label-${NEW_VARIABLE_ID}`,
},
});
});
});
describe('when setCurrentVariableValue is dispatched and current.text is an Array with values', () => {
it('then state should be correct', () => {
const adapter = createQueryVariableAdapter();
......
public/app/features/variables/state/sharedReducer.ts
......@@ -4,7 +4,7 @@ import { default as lodashDefaults } from 'lodash/defaults';
import { LoadingState, VariableType } from '@grafana/data';
import { VariableModel, VariableOption, VariableWithOptions } from '../types';
import { AddVariable, getInstanceState, NEW_VARIABLE_ID, VariablePayload } from './types';
import { AddVariable, getInstanceState, VariablePayload } from './types';
import { variableAdapters } from '../adapters';
import { changeVariableNameSucceeded } from '../editor/reducer';
import { initialVariablesState, VariablesState } from './variablesReducer';
......@@ -96,16 +96,6 @@ const sharedReducerSlice = createSlice({
state[toVariable.id].index = action.payload.data.fromIndex;
}
},
storeNewVariable: (state: VariablesState, action: PayloadAction<VariablePayload>) => {
const id = action.payload.id;
const emptyVariable = cloneDeep<VariableModel>(state[NEW_VARIABLE_ID]);
state[id] = {
...cloneDeep(variableAdapters.get(action.payload.type).initialState),
...emptyVariable,
id,
index: emptyVariable.index,
};
},
changeVariableType: (state: VariablesState, action: PayloadAction<VariablePayload<{ newType: VariableType }>>) => {
const { id } = action.payload;
const { label, name, index } = state[id];
......@@ -182,7 +172,6 @@ export const {
addVariable,
changeVariableProp,
changeVariableOrder,
storeNewVariable,
duplicateVariable,
setCurrentVariableValue,
changeVariableType,
......
public/app/features/variables/utils.ts
import isString from 'lodash/isString';
import { ScopedVars } from '@grafana/data';
import { ScopedVars, VariableType } from '@grafana/data';
import { getTemplateSrv } from '@grafana/runtime';
import { ALL_VARIABLE_TEXT } from './state/types';
import { QueryVariableModel, VariableModel, VariableRefresh } from './types';
import { getTimeSrv } from '../dashboard/services/TimeSrv';
import { variableAdapters } from './adapters';
/*
* This regex matches 3 types of variable reference with an optional format specifier
......@@ -146,3 +147,13 @@ export function getVariableRefresh(variable: VariableModel): VariableRefresh {
return queryVariable.refresh;
}
export function getVariableTypes(): Array<{ label: string; value: VariableType }> {
return variableAdapters
.list()
.filter(v => v.id !== 'system')
.map(({ id, name }) => ({
label: name,
value: id,
}));
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment