Skip to content

Commit 2c04099

Browse files
committed
Backward compatible
Signed-off-by: Guillaume LADORME <Gladorme@users.noreply.github.com>
1 parent eb2f80f commit 2c04099

9 files changed

Lines changed: 118 additions & 64 deletions

File tree

timeserieschart/schemas/migrate/migrate.cue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ spec: {
192192
// migrate fixedColor overrides to querySettings when applicable
193193
#querySettings: [
194194
for i, target in (*#panel.targets | []) {
195-
queryIndex: "Query #\(i+1)"
195+
queryName: "Query #\(i+1)"
196196
for override in (*#panel.fieldConfig.overrides | [])
197197
if (override.matcher.id == "byName" || override.matcher.id == "byRegexp" || override.matcher.id == "byFrameRefID") && override.matcher.options != _|_
198198
for property in override.properties

timeserieschart/schemas/migrate/tests/color-based-on-legend-text/expected.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,28 @@
77
},
88
"querySettings": [
99
{
10-
"queryIndex": "Query #1",
10+
"queryName": "Query #1",
1111
"colorMode": "fixed",
1212
"colorValue": "#EAB839"
1313
},
1414
{
15-
"queryIndex": "Query #2",
15+
"queryName": "Query #2",
1616
"colorMode": "fixed",
1717
"colorValue": "#0A437C"
1818
},
1919
{
20-
"queryIndex": "Query #3",
20+
"queryName": "Query #3",
2121
"colorMode": "fixed",
2222
"colorValue": "#890F02",
2323
"areaOpacity": 1
2424
},
2525
{
26-
"queryIndex": "Query #5",
26+
"queryName": "Query #5",
2727
"colorMode": "fixed",
2828
"colorValue": "#6D1F62"
2929
},
3030
{
31-
"queryIndex": "Query #6",
31+
"queryName": "Query #6",
3232
"colorMode": "fixed",
3333
"colorValue": "#052B51"
3434
}

timeserieschart/schemas/migrate/tests/multiple-overrides/expected.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,25 @@
2121
},
2222
"querySettings": [
2323
{
24-
"queryIndex": "Query #2",
24+
"queryName": "Query #2",
2525
"colorMode": "fixed",
2626
"colorValue": "#5794F2",
2727
"lineStyle": "dashed",
2828
"areaOpacity": 0
2929
},
3030
{
31-
"queryIndex": "Query #3",
31+
"queryName": "Query #3",
3232
"colorMode": "fixed",
3333
"colorValue": "#F2495C",
3434
"areaOpacity": 0
3535
},
3636
{
37-
"queryIndex": "Query #4",
37+
"queryName": "Query #4",
3838
"colorMode": "fixed",
3939
"colorValue": "#3274D9"
4040
},
4141
{
42-
"queryIndex": "Query #5",
42+
"queryName": "Query #5",
4343
"colorMode": "fixed",
4444
"colorValue": "#fade2a",
4545
"lineStyle": "dashed",

timeserieschart/schemas/time-series.cue

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ spec: close({
6262
}
6363

6464
#querySettings: [...{
65-
queryIndex: strings.MinRunes(1)
65+
queryName?: strings.MinRunes(1)
66+
// queryIndex is deprecated, use queryName instead. Kept for backward compatibility.
67+
queryIndex?: int & >=0
6668
colorMode?: "fixed" | "fixed-single" // NB: "palette" could be added later
6769
colorValue?: =~"^#(?:[0-9a-fA-F]{3}){1,2}$" // hexadecimal color code
6870
lineStyle?: #lineStyle

timeserieschart/sdk/go/time-series.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,12 @@ const (
121121
)
122122

123123
type QuerySettingsItem struct {
124-
QueryIndex string `json:"queryIndex" yaml:"queryIndex"`
124+
// QueryName is the preferred way to target a query.
125+
QueryName string `json:"queryName,omitempty" yaml:"queryName,omitempty"`
126+
// QueryIndex is deprecated, use QueryName instead. Kept for backward compatibility.
127+
//
128+
// Deprecated: use QueryName instead.
129+
QueryIndex uint `json:"queryIndex,omitempty" yaml:"queryIndex,omitempty"`
125130
ColorMode ColorMode `json:"colorMode,omitempty" yaml:"colorMode,omitempty"`
126131
ColorValue string `json:"colorValue,omitempty" yaml:"colorValue,omitempty"`
127132
LineStyle string `json:"lineStyle,omitempty" yaml:"lineStyle,omitempty"`

timeserieschart/src/QuerySettingsEditor.tsx

Lines changed: 79 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import DeleteIcon from 'mdi-material-ui/DeleteOutline';
3131
import AddIcon from 'mdi-material-ui/Plus';
3232
import CloseIcon from 'mdi-material-ui/Close';
3333
import { produce } from 'immer';
34-
import { generateQueryNames, useDataQueriesContext } from '@perses-dev/plugin-system';
34+
import { generateQueryNames, useDataQueriesContext, defaultQueryName } from '@perses-dev/plugin-system';
3535
import {
3636
DEFAULT_AREA_OPACITY,
3737
LINE_STYLE_CONFIG,
@@ -42,35 +42,70 @@ import {
4242
} from './time-series-chart-model';
4343

4444
const DEFAULT_COLOR_VALUE = '#555';
45-
const NO_INDEX_AVAILABLE = '-1'; // invalid array index value used to represent the fact that no query index is available
45+
const NO_QUERY_AVAILABLE = '-1'; // sentinel value used to represent the fact that no query is available
46+
47+
/**
48+
* Resolves the effective query name from a settings entry.
49+
* Falls back to the deprecated `queryIndex` field when `queryName` is absent.
50+
*/
51+
function resolveQueryName(querySettings: QuerySettingsOptions, queryNames: string[]): string {
52+
if (querySettings.queryName !== undefined) {
53+
return querySettings.queryName;
54+
}
55+
if (querySettings.queryIndex !== undefined) {
56+
return queryNames[querySettings.queryIndex] ?? defaultQueryName(querySettings.queryIndex);
57+
}
58+
return '';
59+
}
60+
61+
/**
62+
* Normalizes a settings entry so it always persists via `queryName`.
63+
* Converts the deprecated `queryIndex` to `queryName` and removes the old field.
64+
*/
65+
function normalizeToQueryName(querySettings: QuerySettingsOptions, queryNames: string[]): QuerySettingsOptions {
66+
if (querySettings.queryIndex === undefined) {
67+
return querySettings;
68+
}
69+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
70+
const { queryIndex, ...rest } = querySettings;
71+
return { ...rest, queryName: rest.queryName ?? queryNames[queryIndex] ?? defaultQueryName(queryIndex) };
72+
}
4673

4774
export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): ReactElement {
4875
const { onChange, value } = props;
4976
const querySettingsList = value.querySettings;
5077

51-
const handleQuerySettingsChange = (newQuerySettings: QuerySettingsOptions[]): void => {
52-
onChange(
53-
produce(value, (draft: TimeSeriesChartOptions) => {
54-
draft.querySettings = newQuerySettings;
55-
})
56-
);
57-
};
58-
// Every time a new query settings input is added, we want to focus the recently added input
78+
// Hooks first — order must be stable across renders
5979
const recentlyAddedInputRef = useRef<HTMLInputElement | null>(null);
6080
const focusRef = useRef(false);
81+
82+
const { queryDefinitions } = useDataQueriesContext();
83+
const queryNames: string[] = useMemo(() => generateQueryNames(queryDefinitions), [queryDefinitions]);
84+
85+
// Every time a new query settings input is added, focus it
6186
useEffect(() => {
6287
if (!recentlyAddedInputRef.current || !focusRef.current) return;
6388
recentlyAddedInputRef.current?.focus();
6489
focusRef.current = false;
6590
}, [querySettingsList?.length]);
6691

67-
const handleQueryIndexChange = (e: React.ChangeEvent<HTMLInputElement>, i: number): void => {
92+
// All writes go through here. Any entry still using the deprecated `queryIndex` is
93+
// normalized to `queryName` at save time — no side-effect migration needed.
94+
const handleQuerySettingsChange = (newQuerySettings: QuerySettingsOptions[]): void => {
95+
onChange(
96+
produce(value, (draft: TimeSeriesChartOptions) => {
97+
draft.querySettings = newQuerySettings.map((qs) => normalizeToQueryName(qs, queryNames));
98+
})
99+
);
100+
};
101+
102+
const handleQueryNameChange = (e: React.ChangeEvent<HTMLInputElement>, i: number): void => {
68103
if (querySettingsList !== undefined) {
69104
handleQuerySettingsChange(
70105
produce(querySettingsList, (draft) => {
71106
const querySettings = draft?.[i];
72107
if (querySettings) {
73-
querySettings.queryIndex = e.target.value;
108+
querySettings.queryName = e.target.value;
74109
}
75110
})
76111
);
@@ -223,24 +258,20 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R
223258
});
224259
};
225260

226-
const { queryDefinitions } = useDataQueriesContext();
227-
228-
const queryIndexes: string[] = useMemo(() => generateQueryNames(queryDefinitions), [queryDefinitions]);
229-
230-
// Compute the list of query indexes for which query settings are not already defined.
231-
// This is to avoid already-booked indexes to still be selectable in the dropdown(s)
232-
const availableQueryIndexes = useMemo(() => {
233-
return queryIndexes.filter((name) => {
234-
return !querySettingsList?.some((qs) => qs.queryIndex === name);
261+
// Compute the list of query names for which query settings are not already defined.
262+
// This is to avoid already-booked queries to still be selectable in the dropdown(s)
263+
const availableQueryNames = useMemo(() => {
264+
return queryNames.filter((name) => {
265+
return !querySettingsList?.some((qs) => resolveQueryName(qs, queryNames) === name);
235266
});
236-
}, [queryIndexes, querySettingsList]);
267+
}, [queryNames, querySettingsList]);
237268

238-
const firstAvailableQueryIndex = useMemo(() => {
239-
return availableQueryIndexes[0] ?? NO_INDEX_AVAILABLE;
240-
}, [availableQueryIndexes]);
269+
const firstAvailableQueryName = useMemo(() => {
270+
return availableQueryNames[0] ?? NO_QUERY_AVAILABLE;
271+
}, [availableQueryNames]);
241272

242273
const defaultQuerySettings: QuerySettingsOptions = {
243-
queryIndex: firstAvailableQueryIndex,
274+
queryName: firstAvailableQueryName,
244275
};
245276

246277
const addQuerySettingsInput = (): void => {
@@ -267,10 +298,11 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R
267298
<QuerySettingsInput
268299
inputRef={i === querySettingsList.length - 1 ? recentlyAddedInputRef : undefined}
269300
key={i}
301+
queryName={resolveQueryName(querySettings, queryNames)}
270302
querySettings={querySettings}
271-
availableQueryIndexes={availableQueryIndexes}
272-
onQueryIndexChange={(e) => {
273-
handleQueryIndexChange(e, i);
303+
availableQueryNames={availableQueryNames}
304+
onQueryNameChange={(e) => {
305+
handleQueryNameChange(e, i);
274306
}}
275307
onColorModeChange={(e) => handleColorModeChange(e, i)}
276308
onColorValueChange={(color) => handleColorValueChange(color, i)}
@@ -291,7 +323,7 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R
291323
/>
292324
))
293325
)}
294-
{queryDefinitions.length > 0 && firstAvailableQueryIndex !== NO_INDEX_AVAILABLE && (
326+
{queryDefinitions.length > 0 && firstAvailableQueryName !== NO_QUERY_AVAILABLE && (
295327
<Button variant="contained" startIcon={<AddIcon />} sx={{ marginTop: 1 }} onClick={addQuerySettingsInput}>
296328
Add Query Settings
297329
</Button>
@@ -302,8 +334,9 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R
302334

303335
interface QuerySettingsInputProps {
304336
querySettings: QuerySettingsOptions;
305-
availableQueryIndexes: string[];
306-
onQueryIndexChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
337+
queryName: string;
338+
availableQueryNames: string[];
339+
onQueryNameChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
307340
onColorModeChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
308341
onColorValueChange: (colorValue: string) => void;
309342
onLineStyleChange: (lineStyle: string) => void;
@@ -323,9 +356,10 @@ interface QuerySettingsInputProps {
323356
}
324357

325358
function QuerySettingsInput({
326-
querySettings: { queryIndex, colorMode, colorValue, lineStyle, areaOpacity, format },
327-
availableQueryIndexes,
328-
onQueryIndexChange,
359+
querySettings: { colorMode, colorValue, lineStyle, areaOpacity, format },
360+
queryName,
361+
availableQueryNames,
362+
onQueryNameChange,
329363
onColorModeChange,
330364
onColorValueChange,
331365
onLineStyleChange,
@@ -342,8 +376,8 @@ function QuerySettingsInput({
342376
onRemoveFormat,
343377
onFormatChange,
344378
}: QuerySettingsInputProps): ReactElement {
345-
// current query index should also be selectable
346-
const selectableQueryIndex = availableQueryIndexes.sort((a, b) => a.localeCompare(b));
379+
// current query name should also be selectable
380+
const selectableQueryNames = availableQueryNames.slice().sort((a, b) => a.localeCompare(b));
347381

348382
// State for dropdown menu
349383
const [anchorEl, setAnchorEl] = useState<null | HTMLElement>(null);
@@ -394,15 +428,15 @@ function QuerySettingsInput({
394428
<TextField
395429
select
396430
inputRef={inputRef}
397-
value={queryIndex}
431+
value={queryName}
398432
label="Query"
399-
onChange={onQueryIndexChange}
433+
onChange={onQueryNameChange}
400434
sx={{ minWidth: '75px' }}
401435
>
402-
<MenuItem value={queryIndex}>{queryIndex}</MenuItem>
403-
{selectableQueryIndex.map((qi) => (
404-
<MenuItem key={`query-${qi}`} value={qi}>
405-
{qi}
436+
<MenuItem value={queryName}>{queryName}</MenuItem>
437+
{selectableQueryNames.map((qn) => (
438+
<MenuItem key={`query-${qn}`} value={qn}>
439+
{qn}
406440
</MenuItem>
407441
))}
408442
</TextField>
@@ -415,7 +449,7 @@ function QuerySettingsInput({
415449
<MenuItem value="fixed">Fixed</MenuItem>
416450
</TextField>
417451
<OptionsColorPicker
418-
label={queryIndex}
452+
label={queryName}
419453
color={colorValue || DEFAULT_COLOR_VALUE}
420454
onColorChange={onColorValueChange}
421455
/>
@@ -507,7 +541,7 @@ function QuerySettingsInput({
507541
</Stack>
508542
{/* Delete Button for this query settings */}
509543
<Box sx={{ display: 'flex', alignItems: 'center', justifyContent: 'center' }}>
510-
<IconButton aria-label={`delete settings for query '${queryIndex}'`} onClick={onDelete}>
544+
<IconButton aria-label={`delete settings for query '${queryName}'`} onClick={onDelete}>
511545
<DeleteIcon />
512546
</IconButton>
513547
</Box>

timeserieschart/src/TimeSeriesChartPanel.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,15 @@ export function TimeSeriesChartPanel(props: TimeSeriesChartProps): ReactElement
212212
// queries & querySettings indices do not necessarily match, so we have to check the tail value of the $ref attribute
213213
let querySettings: QuerySettingsOptions | undefined;
214214
for (const item of querySettingsList ?? []) {
215-
if (item.queryIndex === result.definition.spec.name || item.queryIndex === defaultQueryName(queryIndex)) {
215+
const matchesByName =
216+
item.queryName !== undefined &&
217+
(item.queryName === result.definition.spec.name || item.queryName === defaultQueryName(queryIndex));
218+
// queryIndex is deprecated but still supported for backward compatibility.
219+
const matchesByIndex = item.queryIndex !== undefined && item.queryIndex === queryIndex;
220+
if (matchesByName || matchesByIndex) {
216221
querySettings = item;
217222
// We don't break the loop here just in case there are multiple querySettings defined for the
218-
// same queryIndex, because in that case we want the last one to take precedence.
223+
// same query, because in that case we want the last one to take precedence.
219224
}
220225
}
221226

timeserieschart/src/time-series-chart-model.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,15 @@ export interface TimeSeriesChartOptions {
4040
}
4141

4242
export interface QuerySettingsOptions {
43-
queryIndex: string;
43+
/**
44+
* Name of the query these settings apply to. This is the preferred way to target a query.
45+
*/
46+
queryName?: string;
47+
/**
48+
* @deprecated Use `queryName` instead. Kept for backward compatibility with existing
49+
* configurations. When loaded in the editor, `queryIndex` is automatically migrated to `queryName`.
50+
*/
51+
queryIndex?: number;
4452
colorMode?: 'fixed' | 'fixed-single';
4553
colorValue?: string;
4654
lineStyle?: LineStyleType;

timeserieschart/src/utils/palette-gen.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ describe('getSeriesColor', () => {
120120
seriesName: testSeriesName,
121121
seriesIndex: 0,
122122
querySettings: {
123-
queryIndex: defaultQueryName(0),
123+
queryName: defaultQueryName(0),
124124
colorMode: 'fixed',
125125
colorValue: '#000',
126126
},
@@ -143,7 +143,7 @@ describe('getSeriesColor', () => {
143143
seriesName: testSeriesName,
144144
seriesIndex: 0,
145145
querySettings: {
146-
queryIndex: defaultQueryName(0),
146+
queryName: defaultQueryName(0),
147147
colorMode: 'fixed',
148148
colorValue: '#000',
149149
},
@@ -166,7 +166,7 @@ describe('getSeriesColor', () => {
166166
seriesName: testSeriesName,
167167
seriesIndex: 0,
168168
querySettings: {
169-
queryIndex: defaultQueryName(0),
169+
queryName: defaultQueryName(0),
170170
colorMode: 'fixed-single',
171171
colorValue: '#000',
172172
},
@@ -189,7 +189,7 @@ describe('getSeriesColor', () => {
189189
seriesName: testSeriesName,
190190
seriesIndex: 0,
191191
querySettings: {
192-
queryIndex: defaultQueryName(0),
192+
queryName: defaultQueryName(0),
193193
colorMode: 'fixed-single',
194194
colorValue: '#000',
195195
},

0 commit comments

Comments
 (0)