Commit a7796229 by Marcus Andersson Committed by GitHub

Variables: prevent adhoc filters from crashing when they are not loaded properly (#28226)

* fixing so we will present a better error message when loading ad hoc filter variables.

* added tests to verify url parsing.

* added a test to make sure it works the oppisite way.
parent 65940c77
import React, { useState } from 'react';
import React from 'react';
import { cx } from 'emotion';
import _ from 'lodash';
import { SegmentSelect } from './SegmentSelect';
import { SelectableValue } from '@grafana/data';
import { useExpandableLabel, SegmentProps } from '.';
import { useAsyncFn } from 'react-use';
import { AsyncState } from 'react-use/lib/useAsync';
export interface SegmentAsyncProps<T> extends SegmentProps<T> {
value?: T | SelectableValue<T>;
......@@ -20,20 +22,14 @@ export function SegmentAsync<T>({
allowCustomValue,
placeholder,
}: React.PropsWithChildren<SegmentAsyncProps<T>>) {
const [selectPlaceholder, setSelectPlaceholder] = useState<string>('');
const [loadedOptions, setLoadedOptions] = useState<Array<SelectableValue<T>>>([]);
const [state, fetchOptions] = useAsyncFn(loadOptions, [loadOptions]);
const [Label, width, expanded, setExpanded] = useExpandableLabel(false);
if (!expanded) {
const label = _.isObject(value) ? value.label : value;
return (
<Label
onClick={async () => {
setSelectPlaceholder('Loading options...');
const opts = await loadOptions();
setLoadedOptions(opts);
setSelectPlaceholder(opts.length ? '' : 'No options found');
}}
onClick={fetchOptions}
Component={
Component || (
<a className={cx('gf-form-label', 'query-part', !value && placeholder && 'query-placeholder', className)}>
......@@ -48,21 +44,33 @@ export function SegmentAsync<T>({
return (
<SegmentSelect
value={value && !_.isObject(value) ? { value } : value}
options={loadedOptions}
options={state.value ?? []}
width={width}
noOptionsMessage={selectPlaceholder}
noOptionsMessage={mapStateToNoOptionsMessage(state)}
allowCustomValue={allowCustomValue}
onClickOutside={() => {
setSelectPlaceholder('');
setLoadedOptions([]);
setExpanded(false);
}}
onChange={item => {
setSelectPlaceholder('');
setLoadedOptions([]);
setExpanded(false);
onChange(item);
}}
/>
);
}
function mapStateToNoOptionsMessage<T>(state: AsyncState<Array<SelectableValue<T>>>): string {
if (state.loading) {
return 'Loading options...';
}
if (state.error) {
return 'Failed to load options';
}
if (!Array.isArray(state.value) || state.value.length === 0) {
return 'No options found';
}
return '';
}
......@@ -42,6 +42,42 @@ describe('urlParser', () => {
});
});
describe('parsing toUrl with filters without values', () => {
it('then url params should be correct', () => {
const a: AdHocVariableFilter = {
value: '',
key: 'key',
operator: '',
condition: '',
};
const filters: AdHocVariableFilter[] = [a];
const expectedA = `key||`;
const expected: string[] = [expectedA];
expect(toUrl(filters)).toEqual(expected);
});
});
describe('parsing toUrl with filters with undefined values', () => {
it('then url params should be correct', () => {
const a = ({
value: undefined,
key: 'key',
operator: undefined,
condition: '',
} as unknown) as AdHocVariableFilter;
const filters: AdHocVariableFilter[] = [a];
const expectedA = `key||`;
const expected: string[] = [expectedA];
expect(toUrl(filters)).toEqual(expected);
});
});
describe('parsing toFilters with url containing no filters as string', () => {
it('then url params should be correct', () => {
const url: UrlQueryValue = '';
......@@ -89,6 +125,30 @@ describe('urlParser', () => {
expect(toFilters(url)).toEqual(expected);
});
});
describe('parsing toFilters with url containing filter with empty values', () => {
it('then url params should be correct', () => {
const url: UrlQueryValue = 'key||';
const expected: AdHocVariableFilter[] = [
{
value: '',
key: 'key',
operator: '',
condition: '',
},
];
expect(toFilters(url)).toEqual(expected);
});
});
describe('parsing toFilters with url containing no filters as string', () => {
it('then url params should be correct', () => {
const url: UrlQueryValue = '';
const expected: AdHocVariableFilter[] = [];
expect(toFilters(url)).toEqual(expected);
});
});
});
function createFilter(value: string, operator = '='): AdHocVariableFilter {
......
......@@ -20,12 +20,12 @@ export const toFilters = (value: UrlQueryValue): AdHocVariableFilter[] => {
return filter === null ? [] : [filter];
};
function escapeDelimiter(value: string) {
return value.replace(/\|/g, '__gfp__');
function escapeDelimiter(value: string | undefined): string {
return value?.replace(/\|/g, '__gfp__') ?? '';
}
function unescapeDelimiter(value: string) {
return value.replace(/__gfp__/g, '|');
function unescapeDelimiter(value: string | undefined): string {
return value?.replace(/__gfp__/g, '|') ?? '';
}
function toArray(filter: AdHocVariableFilter): string[] {
......
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