Skip to content

Commit

Permalink
Remove urlOptions
Browse files Browse the repository at this point in the history
  • Loading branch information
@bfmatei
bfmatei committed May 10, 2024
1 parent b223396 commit 97eba21
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 38 deletions.
9 changes: 9 additions & 0 deletions packages/scenes/src/variables/groupby/GroupByVariable.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('GroupByVariable', () => {
});

expect(variable.state.value).toEqual('');
expect(variable.state.text).toEqual('');

act(() => {
variable.changeValueTo(['a']);
Expand All @@ -68,12 +69,14 @@ describe('GroupByVariable', () => {
});

expect(variable.state.value).toEqual(['a', 'b']);
expect(variable.state.text).toEqual(['a', 'b']);

act(() => {
locationService.push('/?var-test=a&var-test=b&var-test=c');
});

expect(variable.state.value).toEqual(['a', 'b', 'c']);
expect(variable.state.text).toEqual(['a', 'b', 'c']);
});

it('should work with received options', async () => {
Expand All @@ -95,6 +98,7 @@ describe('GroupByVariable', () => {
});

expect(variable.state.value).toEqual('');
expect(variable.state.text).toEqual('');

act(() => {
variable.changeValueTo(['a']);
Expand All @@ -107,12 +111,14 @@ describe('GroupByVariable', () => {
});

expect(variable.state.value).toEqual(['a', 'b']);
expect(variable.state.text).toEqual(['A', 'b']);

act(() => {
locationService.push('/?var-test=a,A&var-test=b&var-test=c');
});

expect(variable.state.value).toEqual(['a', 'b', 'c']);
expect(variable.state.text).toEqual(['A', 'b', 'c']);
});

it('should work with commas', async () => {
Expand All @@ -124,6 +130,7 @@ describe('GroupByVariable', () => {
});

expect(variable.state.value).toEqual('');
expect(variable.state.text).toEqual('');

await act(async () => {
await lastValueFrom(variable.validateAndUpdate());
Expand All @@ -140,6 +147,7 @@ describe('GroupByVariable', () => {
});

expect(variable.state.value).toEqual(['a', 'b,something']);
expect(variable.state.text).toEqual(['A,something', 'b,something']);

act(() => {
locationService.push(
Expand All @@ -148,6 +156,7 @@ describe('GroupByVariable', () => {
});

expect(variable.state.value).toEqual(['a', 'b,something', 'c,something']);
expect(variable.state.text).toEqual(['A,something', 'b,something', 'C,something']);
});
});

Expand Down
15 changes: 7 additions & 8 deletions packages/scenes/src/variables/groupby/GroupByVariable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { MultiValueVariable, MultiValueVariableState, VariableGetOptionsArgs } f
import { from, lastValueFrom, map, mergeMap, Observable, of, take } from 'rxjs';
import { getDataSource } from '../../utils/getDataSource';
import { InputActionMeta, MultiSelect } from '@grafana/ui';
import { isArray, unionBy } from 'lodash';
import { isArray } from 'lodash';
import { getQueriesForVariables } from '../utils';
import { GroupByVariableUrlSyncHandler } from './GroupByVariableUrlSyncHandler';

Expand All @@ -19,8 +19,6 @@ export interface GroupByVariableState extends MultiValueVariableState {
/** The visible keys to group on */
// TODO review this type and name (naming is hard)
defaultOptions?: MetricFindValue[];
/** Options obtained through URL sync */
urlOptions?: MetricFindValue[];
/** Base filters to always apply when looking up keys */
baseFilters?: AdHocVariableFilter[];
/** Datasource to use for getTagKeys and also controls which scene queries the group by should apply to */
Expand Down Expand Up @@ -84,12 +82,10 @@ export class GroupByVariable extends MultiValueVariable<GroupByVariableState> {
}

public getValueOptions(args: VariableGetOptionsArgs): Observable<VariableValueOption[]> {
const urlOptions = this.state.urlOptions ?? [];

// When default dimensions are provided, return the static list
if (this.state.defaultOptions) {
return of(
unionBy(this.state.defaultOptions, urlOptions, 'value').map((o) => ({
this.state.defaultOptions.map((o) => ({
label: o.text,
value: String(o.value),
}))
Expand All @@ -107,7 +103,7 @@ export class GroupByVariable extends MultiValueVariable<GroupByVariableState> {
return from(this._getKeys(ds)).pipe(
take(1),
mergeMap((data: MetricFindValue[]) => {
const a: VariableValueOption[] = unionBy(data, urlOptions, 'value').map((i) => {
const a: VariableValueOption[] = data.map((i) => {
return {
label: i.text,
value: i.value ? String(i.value) : i.text,
Expand Down Expand Up @@ -239,7 +235,10 @@ export function GroupByVariableRenderer({ model }: SceneComponentProps<MultiValu
isLoading={isFetchingOptions}
onInputChange={onInputChange}
onBlur={() => {
model.changeValueTo(uncommittedValue.map((x) => x.value!));
model.changeValueTo(
uncommittedValue.map((x) => x.value!),
uncommittedValue.map((x) => x.label!)
);
}}
onChange={(newValue, action) => {
if (action.action === 'clear' && noValueOnClear) {
Expand Down
66 changes: 36 additions & 30 deletions packages/scenes/src/variables/groupby/GroupByVariableUrlSyncHandler.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { unzip, zip } from 'lodash';
import { SceneObjectUrlSyncHandler, SceneObjectUrlValues } from '../../core/types';
import { GroupByVariable } from './GroupByVariable';
import { toUrlCommaDelimitedString, unescapeUrlDelimiters } from '../utils';
import { VariableValueSingle } from '../types';
import { VariableValue } from '../types';

export class GroupByVariableUrlSyncHandler implements SceneObjectUrlSyncHandler {
public constructor(private _sceneObject: GroupByVariable) {}
Expand All @@ -24,14 +23,7 @@ export class GroupByVariableUrlSyncHandler implements SceneObjectUrlSyncHandler
return {};
}

let { value: values, text: texts } = this._sceneObject.state;

values = Array.isArray(values) ? values : [values];
texts = Array.isArray(texts) ? texts : [texts];

const urlValue = zip(values, texts).map(toUrlValues);

return { [this.getKey()]: urlValue };
return { [this.getKey()]: toUrlValues(this._sceneObject.state.value, this._sceneObject.state.text) };
}

public updateFromUrl(values: SceneObjectUrlValues): void {
Expand All @@ -46,32 +38,46 @@ export class GroupByVariableUrlSyncHandler implements SceneObjectUrlSyncHandler
this._sceneObject.skipNextValidation = true;
}

urlValue = Array.isArray(urlValue) ? urlValue : [urlValue];
const valuesLabelsPairs = urlValue.map((value) => (value ? value.split(',') : [value]));
let [values, labels] = unzip(valuesLabelsPairs);
const { values, texts } = fromUrlValues(urlValue);

values = (values ?? []).map(unescapeUrlDelimiters);
labels = (labels ?? []).map(unescapeUrlDelimiters);
this._sceneObject.changeValueTo(values, texts);
}
}
}

this._sceneObject.setState({
urlOptions: values.map((value, idx) => ({
value,
text: labels[idx],
})),
});
function toUrlValues(values: VariableValue, texts: VariableValue): string[] {
values = Array.isArray(values) ? values : [values];
texts = Array.isArray(texts) ? texts : [texts];

this._sceneObject.changeValueTo(values, labels);
return values.map((value, idx) => {
if (value === undefined || value === null) {
return '';
}
}

value = String(value);

let text = texts[idx];
text = text === undefined || text === null ? value : String(text);

return toUrlCommaDelimitedString(value, text);
});
}

function toUrlValues([value, label]: [VariableValueSingle | undefined, VariableValueSingle | undefined]): string {
if (value === undefined || value === null) {
return '';
}
function fromUrlValues(urlValues: string | string[]): { values: string[]; texts: string[] } {
urlValues = Array.isArray(urlValues) ? urlValues : [urlValues];

return urlValues.reduce(
(acc, urlValue) => {
const [value, label] = (urlValue ?? '').split(',');

value = String(value);
label = label === undefined || label === null ? value : String(label);
acc.values.push(unescapeUrlDelimiters(value));
acc.texts.push(unescapeUrlDelimiters(label ?? value));

return toUrlCommaDelimitedString(value, label);
return acc;
},
{
values: [],
texts: [],
}
);
}

0 comments on commit 97eba21

Please sign in to comment.