diff --git a/timeserieschart/schemas/migrate/migrate.cue b/timeserieschart/schemas/migrate/migrate.cue index 91ad205f5..a6e878bac 100644 --- a/timeserieschart/schemas/migrate/migrate.cue +++ b/timeserieschart/schemas/migrate/migrate.cue @@ -192,7 +192,7 @@ spec: { // migrate fixedColor overrides to querySettings when applicable #querySettings: [ for i, target in (*#panel.targets | []) { - queryIndex: i + queryName: "Query #\(i+1)" for override in (*#panel.fieldConfig.overrides | []) if (override.matcher.id == "byName" || override.matcher.id == "byRegexp" || override.matcher.id == "byFrameRefID") && override.matcher.options != _|_ for property in override.properties diff --git a/timeserieschart/schemas/migrate/tests/color-based-on-legend-text/expected.json b/timeserieschart/schemas/migrate/tests/color-based-on-legend-text/expected.json index 671679d66..c092160fa 100644 --- a/timeserieschart/schemas/migrate/tests/color-based-on-legend-text/expected.json +++ b/timeserieschart/schemas/migrate/tests/color-based-on-legend-text/expected.json @@ -7,28 +7,28 @@ }, "querySettings": [ { - "queryIndex": 0, + "queryName": "Query #1", "colorMode": "fixed", "colorValue": "#EAB839" }, { - "queryIndex": 1, + "queryName": "Query #2", "colorMode": "fixed", "colorValue": "#0A437C" }, { - "queryIndex": 2, + "queryName": "Query #3", "colorMode": "fixed", "colorValue": "#890F02", "areaOpacity": 1 }, { - "queryIndex": 4, + "queryName": "Query #5", "colorMode": "fixed", "colorValue": "#6D1F62" }, { - "queryIndex": 5, + "queryName": "Query #6", "colorMode": "fixed", "colorValue": "#052B51" } @@ -47,4 +47,4 @@ "min": 0 } } -} \ No newline at end of file +} diff --git a/timeserieschart/schemas/migrate/tests/multiple-overrides/expected.json b/timeserieschart/schemas/migrate/tests/multiple-overrides/expected.json index decea5929..980d4b910 100644 --- a/timeserieschart/schemas/migrate/tests/multiple-overrides/expected.json +++ b/timeserieschart/schemas/migrate/tests/multiple-overrides/expected.json @@ -4,11 +4,7 @@ "legend": { "position": "right", "mode": "table", - "values": [ - "min", - "max", - "mean" - ] + "values": ["min", "max", "mean"] }, "yAxis": { "format": { @@ -25,25 +21,25 @@ }, "querySettings": [ { - "queryIndex": 1, + "queryName": "Query #2", "colorMode": "fixed", "colorValue": "#5794F2", "lineStyle": "dashed", "areaOpacity": 0 }, { - "queryIndex": 2, + "queryName": "Query #3", "colorMode": "fixed", "colorValue": "#F2495C", "areaOpacity": 0 }, { - "queryIndex": 3, + "queryName": "Query #4", "colorMode": "fixed", "colorValue": "#3274D9" }, { - "queryIndex": 4, + "queryName": "Query #5", "colorMode": "fixed", "colorValue": "#fade2a", "lineStyle": "dashed", diff --git a/timeserieschart/schemas/time-series.cue b/timeserieschart/schemas/time-series.cue index 80bdc7c38..6345fd206 100644 --- a/timeserieschart/schemas/time-series.cue +++ b/timeserieschart/schemas/time-series.cue @@ -14,6 +14,8 @@ package model import ( + "strings" + "github.com/perses/shared/cue/common" ) @@ -60,7 +62,9 @@ spec: close({ } #querySettings: [...{ - queryIndex: int & >=0 + queryName?: strings.MinRunes(1) + // queryIndex is deprecated, use queryName instead. Kept for backward compatibility. + queryIndex?: int & >=0 colorMode?: "fixed" | "fixed-single" // NB: "palette" could be added later colorValue?: =~"^#(?:[0-9a-fA-F]{3}){1,2}$" // hexadecimal color code lineStyle?: #lineStyle diff --git a/timeserieschart/sdk/go/time-series.go b/timeserieschart/sdk/go/time-series.go index 3f63ba1c9..d49f41dab 100644 --- a/timeserieschart/sdk/go/time-series.go +++ b/timeserieschart/sdk/go/time-series.go @@ -121,7 +121,9 @@ const ( ) type QuerySettingsItem struct { - QueryIndex uint `json:"queryIndex" yaml:"queryIndex"` + QueryName string `json:"queryName,omitempty" yaml:"queryName,omitempty"` + // Deprecated: use QueryName instead. + QueryIndex uint `json:"queryIndex,omitempty" yaml:"queryIndex,omitempty"` ColorMode ColorMode `json:"colorMode,omitempty" yaml:"colorMode,omitempty"` ColorValue string `json:"colorValue,omitempty" yaml:"colorValue,omitempty"` LineStyle string `json:"lineStyle,omitempty" yaml:"lineStyle,omitempty"` diff --git a/timeserieschart/src/QuerySettingsEditor.tsx b/timeserieschart/src/QuerySettingsEditor.tsx index d8e649904..b2d363394 100644 --- a/timeserieschart/src/QuerySettingsEditor.tsx +++ b/timeserieschart/src/QuerySettingsEditor.tsx @@ -31,7 +31,7 @@ import DeleteIcon from 'mdi-material-ui/DeleteOutline'; import AddIcon from 'mdi-material-ui/Plus'; import CloseIcon from 'mdi-material-ui/Close'; import { produce } from 'immer'; -import { useDataQueriesContext } from '@perses-dev/plugin-system'; +import { generateQueryNames, useDataQueriesContext, defaultQueryName } from '@perses-dev/plugin-system'; import { DEFAULT_AREA_OPACITY, LINE_STYLE_CONFIG, @@ -42,35 +42,68 @@ import { } from './time-series-chart-model'; const DEFAULT_COLOR_VALUE = '#555'; -const NO_INDEX_AVAILABLE = -1; // invalid array index value used to represent the fact that no query index is available +const NO_QUERY_AVAILABLE = '-1'; // sentinel value used to represent the fact that no query is available + +/** + * Resolves the effective query name from a settings entry. + * Falls back to the deprecated `queryIndex` field when `queryName` is absent. + */ +function resolveQueryName(querySettings: QuerySettingsOptions, queryNames: string[]): string { + if (querySettings.queryName !== undefined) { + return querySettings.queryName; + } + if (querySettings.queryIndex !== undefined) { + return queryNames[querySettings.queryIndex] ?? defaultQueryName(querySettings.queryIndex); + } + return ''; +} + +/** + * Normalizes a settings entry so it always persists via `queryName`. + * Converts the deprecated `queryIndex` to `queryName` and removes the old field. + */ +function normalizeToQueryName(querySettings: QuerySettingsOptions, queryNames: string[]): QuerySettingsOptions { + if (querySettings.queryIndex === undefined) { + return querySettings; + } + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { queryIndex, ...rest } = querySettings; + return { ...rest, queryName: rest.queryName ?? queryNames[queryIndex] ?? defaultQueryName(queryIndex) }; +} export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): ReactElement { const { onChange, value } = props; const querySettingsList = value.querySettings; - const handleQuerySettingsChange = (newQuerySettings: QuerySettingsOptions[]): void => { - onChange( - produce(value, (draft: TimeSeriesChartOptions) => { - draft.querySettings = newQuerySettings; - }) - ); - }; - // Every time a new query settings input is added, we want to focus the recently added input const recentlyAddedInputRef = useRef(null); const focusRef = useRef(false); + + const { queryDefinitions } = useDataQueriesContext(); + const queryNames: string[] = useMemo(() => generateQueryNames(queryDefinitions), [queryDefinitions]); + + // Every time a new query settings input is added, focus it useEffect(() => { if (!recentlyAddedInputRef.current || !focusRef.current) return; recentlyAddedInputRef.current?.focus(); focusRef.current = false; }, [querySettingsList?.length]); - const handleQueryIndexChange = (e: React.ChangeEvent, i: number): void => { + // Any entry still using the deprecated `queryIndex` is normalized to `queryName`, remove when queryIndex is removed + const handleQuerySettingsChange = (newQuerySettings: QuerySettingsOptions[]): void => { + onChange( + produce(value, (draft: TimeSeriesChartOptions) => { + draft.querySettings = newQuerySettings.map((qs) => normalizeToQueryName(qs, queryNames)); + }) + ); + }; + + const handleQueryNameChange = (e: React.ChangeEvent, i: number): void => { if (querySettingsList !== undefined) { handleQuerySettingsChange( produce(querySettingsList, (draft) => { const querySettings = draft?.[i]; if (querySettings) { - querySettings.queryIndex = parseInt(e.target.value); + querySettings.queryName = e.target.value; } }) ); @@ -223,23 +256,20 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R }); }; - const { queryDefinitions } = useDataQueriesContext(); - const queryCount = queryDefinitions.length; - - // Compute the list of query indexes for which query settings are not already defined. - // This is to avoid already-booked indexes to still be selectable in the dropdown(s) - const availableQueryIndexes = useMemo(() => { - const bookedQueryIndexes = querySettingsList?.map((querySettings) => querySettings.queryIndex) ?? []; - const allQueryIndexes = Array.from({ length: queryCount }, (_, i) => i); - return allQueryIndexes.filter((_, queryIndex) => !bookedQueryIndexes.includes(queryIndex)); - }, [querySettingsList, queryCount]); + // Compute the list of query names for which query settings are not already defined. + // This is to avoid already-booked queries to still be selectable in the dropdown(s) + const availableQueryNames = useMemo(() => { + return queryNames.filter((name) => { + return !querySettingsList?.some((qs) => resolveQueryName(qs, queryNames) === name); + }); + }, [queryNames, querySettingsList]); - const firstAvailableQueryIndex = useMemo(() => { - return availableQueryIndexes[0] ?? NO_INDEX_AVAILABLE; - }, [availableQueryIndexes]); + const firstAvailableQueryName = useMemo(() => { + return availableQueryNames[0] ?? NO_QUERY_AVAILABLE; + }, [availableQueryNames]); const defaultQuerySettings: QuerySettingsOptions = { - queryIndex: firstAvailableQueryIndex, + queryName: firstAvailableQueryName, }; const addQuerySettingsInput = (): void => { @@ -257,20 +287,20 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R return ( - {queryCount === 0 ? ( + {queryDefinitions.length === 0 ? ( No query defined ) : ( - querySettingsList?.length && - querySettingsList.map((querySettings, i) => ( + querySettingsList?.map((querySettings, i) => ( { - handleQueryIndexChange(e, i); + availableQueryNames={availableQueryNames} + onQueryNameChange={(e) => { + handleQueryNameChange(e, i); }} onColorModeChange={(e) => handleColorModeChange(e, i)} onColorValueChange={(color) => handleColorValueChange(color, i)} @@ -291,7 +321,7 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R /> )) )} - {queryCount > 0 && firstAvailableQueryIndex !== NO_INDEX_AVAILABLE && ( + {queryDefinitions.length > 0 && firstAvailableQueryName !== NO_QUERY_AVAILABLE && ( @@ -302,8 +332,9 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R interface QuerySettingsInputProps { querySettings: QuerySettingsOptions; - availableQueryIndexes: number[]; - onQueryIndexChange: (e: React.ChangeEvent) => void; + queryName: string; + availableQueryNames: string[]; + onQueryNameChange: (e: React.ChangeEvent) => void; onColorModeChange: (e: React.ChangeEvent) => void; onColorValueChange: (colorValue: string) => void; onLineStyleChange: (lineStyle: string) => void; @@ -323,9 +354,10 @@ interface QuerySettingsInputProps { } function QuerySettingsInput({ - querySettings: { queryIndex, colorMode, colorValue, lineStyle, areaOpacity, format }, - availableQueryIndexes, - onQueryIndexChange, + querySettings: { colorMode, colorValue, lineStyle, areaOpacity, format }, + queryName, + availableQueryNames, + onQueryNameChange, onColorModeChange, onColorValueChange, onLineStyleChange, @@ -342,8 +374,8 @@ function QuerySettingsInput({ onRemoveFormat, onFormatChange, }: QuerySettingsInputProps): ReactElement { - // current query index should also be selectable - const selectableQueryIndexes = availableQueryIndexes.concat(queryIndex).sort((a, b) => a - b); + // current query name should also be selectable + const selectableQueryNames = availableQueryNames.slice().sort((a, b) => a.localeCompare(b)); // State for dropdown menu const [anchorEl, setAnchorEl] = useState(null); @@ -394,14 +426,15 @@ function QuerySettingsInput({ - {selectableQueryIndexes.map((qi) => ( - - #{qi + 1} + {queryName} + {selectableQueryNames.map((qn) => ( + + {qn} ))} @@ -414,7 +447,7 @@ function QuerySettingsInput({ Fixed @@ -449,7 +482,7 @@ function QuerySettingsInput({ {/* Area Opacity section */} {areaOpacity !== undefined && ( - {/* Spacer as I don't want to add a prop to SettingsSection for left-padding just for that case.. */} + {/* Spacer as I don't want to add a prop to SettingsSection for left-padding just for that case. */} {/* Delete Button for this query settings */} - + diff --git a/timeserieschart/src/TimeSeriesChartPanel.tsx b/timeserieschart/src/TimeSeriesChartPanel.tsx index b9339f638..0e545feee 100644 --- a/timeserieschart/src/TimeSeriesChartPanel.tsx +++ b/timeserieschart/src/TimeSeriesChartPanel.tsx @@ -23,6 +23,7 @@ import { legendValues, getCalculations, CalculationType, + defaultQueryName, } from '@perses-dev/plugin-system'; import { ChartInstance, @@ -199,15 +200,27 @@ export function TimeSeriesChartPanel(props: TimeSeriesChartProps): ReactElement // TODO: Look into performance optimizations and moving parts of mapping to the lower level chart for (let queryIndex = 0; queryIndex < queryResults.length; queryIndex++) { const result = queryResults[queryIndex]; + if (result === undefined) { + console.warn( + 'Something went wrong with the query result mapping, result is undefined for query index', + queryIndex + ); + continue; + } // Retrieve querySettings for this query, if exists. // queries & querySettings indices do not necessarily match, so we have to check the tail value of the $ref attribute let querySettings: QuerySettingsOptions | undefined; for (const item of querySettingsList ?? []) { - if (item.queryIndex === queryIndex) { + const matchesByName = + item.queryName !== undefined && + (item.queryName === result.definition.spec.name || item.queryName === defaultQueryName(queryIndex)); + // queryIndex is deprecated but still supported for backward compatibility. + const matchesByIndex = item.queryIndex !== undefined && item.queryIndex === queryIndex; + if (matchesByName || matchesByIndex) { querySettings = item; // We don't break the loop here just in case there are multiple querySettings defined for the - // same queryIndex, because in that case we want the last one to take precedence. + // same query, because in that case we want the last one to take precedence. } } @@ -230,7 +243,7 @@ export function TimeSeriesChartPanel(props: TimeSeriesChartProps): ReactElement seriesName: formattedSeriesName, seriesIndex, querySettings: querySettings, - queryHasMultipleResults: (queryResults[queryIndex]?.data?.series?.length ?? 0) > 1, + queryHasMultipleResults: (result.data?.series?.length ?? 0) > 1, }); // We add a unique id for the chart to disambiguate items across charts diff --git a/timeserieschart/src/time-series-chart-model.ts b/timeserieschart/src/time-series-chart-model.ts index d0458b541..3c3f609d4 100644 --- a/timeserieschart/src/time-series-chart-model.ts +++ b/timeserieschart/src/time-series-chart-model.ts @@ -40,7 +40,11 @@ export interface TimeSeriesChartOptions { } export interface QuerySettingsOptions { - queryIndex: number; + queryName?: string; + /** + * @deprecated Use `queryName` instead. + */ + queryIndex?: number; colorMode?: 'fixed' | 'fixed-single'; colorValue?: string; lineStyle?: LineStyleType; diff --git a/timeserieschart/src/utils/palette-gen.test.ts b/timeserieschart/src/utils/palette-gen.test.ts index c1bd6e687..3e8c369d0 100644 --- a/timeserieschart/src/utils/palette-gen.test.ts +++ b/timeserieschart/src/utils/palette-gen.test.ts @@ -11,6 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import { defaultQueryName } from '@perses-dev/plugin-system'; import { TimeSeriesChartVisualOptions } from '../time-series-chart-model'; import { getSeriesColor, getAutoPaletteColor, getCategoricalPaletteColor, SeriesColorProps } from './palette-gen'; @@ -119,7 +120,7 @@ describe('getSeriesColor', () => { seriesName: testSeriesName, seriesIndex: 0, querySettings: { - queryIndex: 0, + queryName: defaultQueryName(0), colorMode: 'fixed', colorValue: '#000', }, @@ -142,7 +143,7 @@ describe('getSeriesColor', () => { seriesName: testSeriesName, seriesIndex: 0, querySettings: { - queryIndex: 0, + queryName: defaultQueryName(0), colorMode: 'fixed', colorValue: '#000', }, @@ -165,7 +166,7 @@ describe('getSeriesColor', () => { seriesName: testSeriesName, seriesIndex: 0, querySettings: { - queryIndex: 0, + queryName: defaultQueryName(0), colorMode: 'fixed-single', colorValue: '#000', }, @@ -188,7 +189,7 @@ describe('getSeriesColor', () => { seriesName: testSeriesName, seriesIndex: 0, querySettings: { - queryIndex: 0, + queryName: defaultQueryName(0), colorMode: 'fixed-single', colorValue: '#000', },