diff --git a/timeserieschart/schemas/migrate/migrate.cue b/timeserieschart/schemas/migrate/migrate.cue index 91ad205f5..d1b51e933 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 + queryIndex: "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..2f0ced90a 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, + "queryIndex": "Query #1", "colorMode": "fixed", "colorValue": "#EAB839" }, { - "queryIndex": 1, + "queryIndex": "Query #2", "colorMode": "fixed", "colorValue": "#0A437C" }, { - "queryIndex": 2, + "queryIndex": "Query #3", "colorMode": "fixed", "colorValue": "#890F02", "areaOpacity": 1 }, { - "queryIndex": 4, + "queryIndex": "Query #5", "colorMode": "fixed", "colorValue": "#6D1F62" }, { - "queryIndex": 5, + "queryIndex": "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..2f6cca8a2 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, + "queryIndex": "Query #2", "colorMode": "fixed", "colorValue": "#5794F2", "lineStyle": "dashed", "areaOpacity": 0 }, { - "queryIndex": 2, + "queryIndex": "Query #3", "colorMode": "fixed", "colorValue": "#F2495C", "areaOpacity": 0 }, { - "queryIndex": 3, + "queryIndex": "Query #4", "colorMode": "fixed", "colorValue": "#3274D9" }, { - "queryIndex": 4, + "queryIndex": "Query #5", "colorMode": "fixed", "colorValue": "#fade2a", "lineStyle": "dashed", diff --git a/timeserieschart/schemas/time-series.cue b/timeserieschart/schemas/time-series.cue index 80bdc7c38..4f05fcec0 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,7 @@ spec: close({ } #querySettings: [...{ - queryIndex: int & >=0 + queryIndex: strings.MinRunes(1) 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..9f50f6c52 100644 --- a/timeserieschart/sdk/go/time-series.go +++ b/timeserieschart/sdk/go/time-series.go @@ -121,7 +121,7 @@ const ( ) type QuerySettingsItem struct { - QueryIndex uint `json:"queryIndex" yaml:"queryIndex"` + QueryIndex string `json:"queryIndex" yaml:"queryIndex"` 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..dd2809a6d 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 } from '@perses-dev/plugin-system'; import { DEFAULT_AREA_OPACITY, LINE_STYLE_CONFIG, @@ -42,7 +42,7 @@ 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_INDEX_AVAILABLE = '-1'; // invalid array index value used to represent the fact that no query index is available export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): ReactElement { const { onChange, value } = props; @@ -70,7 +70,7 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R produce(querySettingsList, (draft) => { const querySettings = draft?.[i]; if (querySettings) { - querySettings.queryIndex = parseInt(e.target.value); + querySettings.queryIndex = e.target.value; } }) ); @@ -224,15 +224,16 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R }; const { queryDefinitions } = useDataQueriesContext(); - const queryCount = queryDefinitions.length; + + const queryIndexes: string[] = useMemo(() => generateQueryNames(queryDefinitions), [queryDefinitions]); // 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]); + return queryIndexes.filter((name) => { + return !querySettingsList?.some((qs) => qs.queryIndex === name); + }); + }, [queryIndexes, querySettingsList]); const firstAvailableQueryIndex = useMemo(() => { return availableQueryIndexes[0] ?? NO_INDEX_AVAILABLE; @@ -257,13 +258,12 @@ 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) => ( )) )} - {queryCount > 0 && firstAvailableQueryIndex !== NO_INDEX_AVAILABLE && ( + {queryDefinitions.length > 0 && firstAvailableQueryIndex !== NO_INDEX_AVAILABLE && ( @@ -302,7 +302,7 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R interface QuerySettingsInputProps { querySettings: QuerySettingsOptions; - availableQueryIndexes: number[]; + availableQueryIndexes: string[]; onQueryIndexChange: (e: React.ChangeEvent) => void; onColorModeChange: (e: React.ChangeEvent) => void; onColorValueChange: (colorValue: string) => void; @@ -343,7 +343,7 @@ function QuerySettingsInput({ onFormatChange, }: QuerySettingsInputProps): ReactElement { // current query index should also be selectable - const selectableQueryIndexes = availableQueryIndexes.concat(queryIndex).sort((a, b) => a - b); + const selectableQueryIndex = availableQueryIndexes.sort((a, b) => a.localeCompare(b)); // State for dropdown menu const [anchorEl, setAnchorEl] = useState(null); @@ -399,9 +399,10 @@ function QuerySettingsInput({ onChange={onQueryIndexChange} sx={{ minWidth: '75px' }} > - {selectableQueryIndexes.map((qi) => ( + {queryIndex} + {selectableQueryIndex.map((qi) => ( - #{qi + 1} + {qi} ))} @@ -414,7 +415,7 @@ function QuerySettingsInput({ Fixed @@ -449,7 +450,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..7ff9ab935 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,12 +200,19 @@ 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) { + if (item.queryIndex === result.definition.spec.name || item.queryIndex === defaultQueryName(queryIndex)) { 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. @@ -230,7 +238,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..4354fa85f 100644 --- a/timeserieschart/src/time-series-chart-model.ts +++ b/timeserieschart/src/time-series-chart-model.ts @@ -40,7 +40,7 @@ export interface TimeSeriesChartOptions { } export interface QuerySettingsOptions { - queryIndex: number; + queryIndex: string; 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..9c183b22d 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, + queryIndex: defaultQueryName(0), colorMode: 'fixed', colorValue: '#000', }, @@ -142,7 +143,7 @@ describe('getSeriesColor', () => { seriesName: testSeriesName, seriesIndex: 0, querySettings: { - queryIndex: 0, + queryIndex: defaultQueryName(0), colorMode: 'fixed', colorValue: '#000', }, @@ -165,7 +166,7 @@ describe('getSeriesColor', () => { seriesName: testSeriesName, seriesIndex: 0, querySettings: { - queryIndex: 0, + queryIndex: defaultQueryName(0), colorMode: 'fixed-single', colorValue: '#000', }, @@ -188,7 +189,7 @@ describe('getSeriesColor', () => { seriesName: testSeriesName, seriesIndex: 0, querySettings: { - queryIndex: 0, + queryIndex: defaultQueryName(0), colorMode: 'fixed-single', colorValue: '#000', },