DashboardScene: Validate variable name in scenes variable editor (#82415)

* add variable name validation

* adjust variable name validation logic

* move variable name validation logic to model

* add tests for onValidateVariableName

* extract variable name validation itest into separate describe
This commit is contained in:
Sergej-Vlasov 2024-02-21 17:16:57 +02:00 committed by GitHub
parent 7e8b679237
commit e1edec02d0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 117 additions and 8 deletions

View File

@ -17,6 +17,7 @@ import {
SceneGridLayout,
VizPanel,
AdHocFiltersVariable,
SceneVariableState,
} from '@grafana/scenes';
import { mockDataSource } from 'app/features/alerting/unified/mocks';
import { LegacyVariableQueryEditor } from 'app/features/variables/editor/LegacyVariableQueryEditor';
@ -206,6 +207,49 @@ describe('VariablesEditView', () => {
});
});
describe('Variables name validation', () => {
let variableView: VariablesEditView;
let variable1: SceneVariableState;
let variable2: SceneVariableState;
beforeAll(async () => {
const result = await buildTestScene();
variableView = result.variableView;
const variables = variableView.getVariables();
variable1 = variables[0].state;
variable2 = variables[1].state;
});
it('should not return error on same name and key', () => {
expect(variableView.onValidateVariableName(variable1.name, variable1.key)[0]).toBe(false);
});
it('should not return error if name is unique', () => {
expect(variableView.onValidateVariableName('unique_variable_name', variable1.key)[0]).toBe(false);
});
it('should return error if global variable name is used', () => {
expect(variableView.onValidateVariableName('__', variable1.key)[0]).toBe(true);
});
it('should not return error if global variable name is used not at the beginning ', () => {
expect(variableView.onValidateVariableName('test__', variable1.key)[0]).toBe(false);
});
it('should return error if name is empty', () => {
expect(variableView.onValidateVariableName('', variable1.key)[0]).toBe(true);
});
it('should return error if non word characters are used', () => {
expect(variableView.onValidateVariableName('-', variable1.key)[0]).toBe(true);
});
it('should return error if variable name is taken', () => {
expect(variableView.onValidateVariableName(variable2.name, variable1.key)[0]).toBe(true);
});
});
describe('Dashboard Variables dependencies', () => {
let variableView: VariablesEditView;
let dashboard: DashboardScene;

View File

@ -12,7 +12,13 @@ import { EditListViewSceneUrlSync } from './EditListViewSceneUrlSync';
import { DashboardEditView, DashboardEditViewState, useDashboardEditPageNav } from './utils';
import { VariableEditorForm } from './variables/VariableEditorForm';
import { VariableEditorList } from './variables/VariableEditorList';
import { EditableVariableType, getVariableDefault, getVariableScene } from './variables/utils';
import {
EditableVariableType,
RESERVED_GLOBAL_VARIABLE_NAME_REGEX,
WORD_CHARACTERS_REGEX,
getVariableDefault,
getVariableScene,
} from './variables/utils';
export interface VariablesEditViewState extends DashboardEditViewState {
editIndex?: number | undefined;
}
@ -168,6 +174,29 @@ export class VariablesEditView extends SceneObjectBase<VariablesEditViewState> i
public onGoBack = () => {
this.setState({ editIndex: undefined });
};
public onValidateVariableName = (name: string, key: string | undefined): [true, string] | [false, null] => {
let errorText = null;
if (!RESERVED_GLOBAL_VARIABLE_NAME_REGEX.test(name)) {
errorText = "Template names cannot begin with '__', that's reserved for Grafana's global variables";
}
if (!WORD_CHARACTERS_REGEX.test(name)) {
errorText = 'Only word characters are allowed in variable names';
}
const variable = this.getVariableSet().getByName(name)?.state;
if (variable && variable.key !== key) {
errorText = 'Variable with the same name already exists';
}
if (errorText) {
return [true, errorText];
}
return [false, null];
};
}
function VariableEditorSettingsListView({ model }: SceneComponentProps<VariablesEditView>) {
@ -190,6 +219,7 @@ function VariableEditorSettingsListView({ model }: SceneComponentProps<Variables
navModel={navModel}
dashboard={dashboard}
onDelete={onDelete}
onValidateVariableName={model.onValidateVariableName}
/>
);
}
@ -218,6 +248,7 @@ interface VariableEditorSettingsEditViewProps {
onTypeChange: (variableType: EditableVariableType) => void;
onGoBack: () => void;
onDelete: (variableName: string) => void;
onValidateVariableName: (name: string, key: string | undefined) => [true, string] | [false, null];
}
function VariableEditorSettingsView({
@ -228,6 +259,7 @@ function VariableEditorSettingsView({
onTypeChange,
onGoBack,
onDelete,
onValidateVariableName,
}: VariableEditorSettingsEditViewProps) {
const parentTab = pageNav.children!.find((p) => p.active)!;
parentTab.parentItem = pageNav;
@ -240,7 +272,13 @@ function VariableEditorSettingsView({
return (
<Page navModel={navModel} pageNav={editVariablePageNav} layout={PageLayoutType.Standard}>
<NavToolbarActions dashboard={dashboard} />
<VariableEditorForm variable={variable} onTypeChange={onTypeChange} onGoBack={onGoBack} onDelete={onDelete} />
<VariableEditorForm
variable={variable}
onTypeChange={onTypeChange}
onGoBack={onGoBack}
onDelete={onDelete}
onValidateVariableName={onValidateVariableName}
/>
</Page>
);
}

View File

@ -1,5 +1,5 @@
import { css } from '@emotion/css';
import React, { FormEvent } from 'react';
import React, { FormEvent, useCallback, useState } from 'react';
import { useAsyncFn } from 'react-use';
import { lastValueFrom } from 'rxjs';
@ -24,23 +24,44 @@ interface VariableEditorFormProps {
onTypeChange: (type: EditableVariableType) => void;
onGoBack: () => void;
onDelete: (variableName: string) => void;
onValidateVariableName: (name: string, key: string | undefined) => [true, string] | [false, null];
}
export function VariableEditorForm({ variable, onTypeChange, onGoBack, onDelete }: VariableEditorFormProps) {
export function VariableEditorForm({
variable,
onTypeChange,
onGoBack,
onDelete,
onValidateVariableName,
}: VariableEditorFormProps) {
const styles = useStyles2(getStyles);
const { name, type, label, description, hide } = variable.useState();
const [nameError, setNameError] = useState<string | null>(null);
const { name, type, label, description, hide, key } = variable.useState();
const EditorToRender = isEditableVariableType(type) ? getVariableEditor(type) : undefined;
const [runQueryState, onRunQuery] = useAsyncFn(async () => {
await lastValueFrom(variable.validateAndUpdate!());
}, [variable]);
const onVariableTypeChange = (option: SelectableValue<EditableVariableType>) => {
if (option.value) {
onTypeChange(option.value);
}
};
const onNameBlur = (e: FormEvent<HTMLInputElement>) => variable.setState({ name: e.currentTarget.value });
const onNameChange = useCallback(
(e: FormEvent<HTMLInputElement>) => {
const [, errorMessage] = onValidateVariableName(e.currentTarget.value, key);
if (nameError !== errorMessage) {
setNameError(errorMessage);
}
},
[key, nameError, onValidateVariableName]
);
const onNameBlur = (e: FormEvent<HTMLInputElement>) => {
if (!nameError) {
variable.setState({ name: e.currentTarget.value });
}
};
const onLabelBlur = (e: FormEvent<HTMLInputElement>) => variable.setState({ label: e.currentTarget.value });
const onDescriptionBlur = (e: FormEvent<HTMLTextAreaElement>) =>
variable.setState({ description: e.currentTarget.value });
@ -64,10 +85,13 @@ export function VariableEditorForm({ variable, onTypeChange, onGoBack, onDelete
description="The name of the template variable. (Max. 50 characters)"
placeholder="Variable name"
defaultValue={name ?? ''}
onChange={onNameChange}
onBlur={onNameBlur}
testId={selectors.pages.Dashboard.Settings.Variables.Edit.General.generalNameInputV2}
maxLength={VariableNameConstraints.MaxSize}
required
invalid={!!nameError}
error={nameError}
/>
<VariableTextField
name="Label"

View File

@ -195,3 +195,6 @@ export function getOptionDataSourceTypes() {
return optionTypes;
}
export const RESERVED_GLOBAL_VARIABLE_NAME_REGEX = /^(?!__).*$/;
export const WORD_CHARACTERS_REGEX = /^\w+$/;