From 97eba21a94948af78b5c45471dc58b9ef0a3cd41 Mon Sep 17 00:00:00 2001 From: Bogdan Matei Date: Fri, 10 May 2024 13:31:47 +0300 Subject: [PATCH] Remove urlOptions --- .../groupby/GroupByVariable.test.tsx | 9 +++ .../src/variables/groupby/GroupByVariable.tsx | 15 ++--- .../groupby/GroupByVariableUrlSyncHandler.ts | 66 ++++++++++--------- 3 files changed, 52 insertions(+), 38 deletions(-) diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx index c74e21d6b..409f6dd26 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx @@ -56,6 +56,7 @@ describe('GroupByVariable', () => { }); expect(variable.state.value).toEqual(''); + expect(variable.state.text).toEqual(''); act(() => { variable.changeValueTo(['a']); @@ -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 () => { @@ -95,6 +98,7 @@ describe('GroupByVariable', () => { }); expect(variable.state.value).toEqual(''); + expect(variable.state.text).toEqual(''); act(() => { variable.changeValueTo(['a']); @@ -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 () => { @@ -124,6 +130,7 @@ describe('GroupByVariable', () => { }); expect(variable.state.value).toEqual(''); + expect(variable.state.text).toEqual(''); await act(async () => { await lastValueFrom(variable.validateAndUpdate()); @@ -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( @@ -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']); }); }); diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 73fce70b8..67b14b45b 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -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'; @@ -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 */ @@ -84,12 +82,10 @@ export class GroupByVariable extends MultiValueVariable { } public getValueOptions(args: VariableGetOptionsArgs): Observable { - 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), })) @@ -107,7 +103,7 @@ export class GroupByVariable extends MultiValueVariable { 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, @@ -239,7 +235,10 @@ export function GroupByVariableRenderer({ model }: SceneComponentProps { - 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) { diff --git a/packages/scenes/src/variables/groupby/GroupByVariableUrlSyncHandler.ts b/packages/scenes/src/variables/groupby/GroupByVariableUrlSyncHandler.ts index a3c43934f..ef936fb04 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariableUrlSyncHandler.ts +++ b/packages/scenes/src/variables/groupby/GroupByVariableUrlSyncHandler.ts @@ -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) {} @@ -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 { @@ -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: [], + } + ); }