Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion timeserieschart/schemas/migrate/migrate.cue
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand All @@ -47,4 +47,4 @@
"min": 0
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@
"legend": {
"position": "right",
"mode": "table",
"values": [
"min",
"max",
"mean"
]
"values": ["min", "max", "mean"]
},
"yAxis": {
"format": {
Expand All @@ -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",
Expand Down
6 changes: 5 additions & 1 deletion timeserieschart/schemas/time-series.cue
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package model

import (
"strings"

"github.com/perses/shared/cue/common"
)

Expand Down Expand Up @@ -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
Comment on lines +65 to 70

@shahrokni shahrokni Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at the following payload that I crafted and sent to the backend.
Since the schema is fully optional now, such a payload is persisted. I think this should be avoided. If we insist that we should keep all fields optional, then maybe we should intercept the req and drop the querySettings completely. (I am just discussing the payload point of view)

Image

Expand Down
4 changes: 3 additions & 1 deletion timeserieschart/sdk/go/time-series.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
129 changes: 81 additions & 48 deletions timeserieschart/src/QuerySettingsEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
Comment on lines +66 to +68

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the idea behind this condition was that, if queryIndex was undefined then queryName would already exist. So, simply return the current one. However, this could go wrong. The QuerySettingsOptions object could be completely empty due to the fact that all fields are optional. Therefore, regardless of how queryName is generated, it is still technically possible to receive an empty object. This may open the door to bugs in future.

/* ALL FIELDS ARE OPTIONAL */
export interface QuerySettingsOptions {
  queryName?: string;
  /**
   * @deprecated Use `queryName` instead.
   */
  queryIndex?: number;
  colorMode?: 'fixed' | 'fixed-single';
  colorValue?: string;
  lineStyle?: LineStyleType;
  areaOpacity?: number;
  format?: FormatOptions;
}

So, maybe?

if (querySettings.queryIndex === undefined && querySettings.queryName!==undefined)

// 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<HTMLInputElement | null>(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<HTMLInputElement>, 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<HTMLInputElement>, 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;
}
})
);
Expand Down Expand Up @@ -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 => {
Expand All @@ -257,20 +287,20 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R

return (
<Stack>
{queryCount === 0 ? (
{queryDefinitions.length === 0 ? (
<Typography mb={2} fontStyle="italic">
No query defined
</Typography>
) : (
querySettingsList?.length &&
querySettingsList.map((querySettings, i) => (
querySettingsList?.map((querySettings, i) => (
<QuerySettingsInput
inputRef={i === querySettingsList.length - 1 ? recentlyAddedInputRef : undefined}
key={i}
queryName={resolveQueryName(querySettings, queryNames)}
querySettings={querySettings}
availableQueryIndexes={availableQueryIndexes}
onQueryIndexChange={(e) => {
handleQueryIndexChange(e, i);
availableQueryNames={availableQueryNames}
onQueryNameChange={(e) => {
handleQueryNameChange(e, i);
}}
onColorModeChange={(e) => handleColorModeChange(e, i)}
onColorValueChange={(color) => handleColorValueChange(color, i)}
Expand All @@ -291,7 +321,7 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R
/>
))
)}
{queryCount > 0 && firstAvailableQueryIndex !== NO_INDEX_AVAILABLE && (
{queryDefinitions.length > 0 && firstAvailableQueryName !== NO_QUERY_AVAILABLE && (
<Button variant="contained" startIcon={<AddIcon />} sx={{ marginTop: 1 }} onClick={addQuerySettingsInput}>
Add Query Settings
</Button>
Expand All @@ -302,8 +332,9 @@ export function QuerySettingsEditor(props: TimeSeriesChartOptionsEditorProps): R

interface QuerySettingsInputProps {
querySettings: QuerySettingsOptions;
availableQueryIndexes: number[];
onQueryIndexChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
queryName: string;
availableQueryNames: string[];
onQueryNameChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
onColorModeChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
onColorValueChange: (colorValue: string) => void;
onLineStyleChange: (lineStyle: string) => void;
Expand All @@ -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,
Expand All @@ -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 | HTMLElement>(null);
Expand Down Expand Up @@ -394,14 +426,15 @@ function QuerySettingsInput({
<TextField
select
inputRef={inputRef}
value={queryIndex}
value={queryName}
label="Query"
onChange={onQueryIndexChange}
onChange={onQueryNameChange}
sx={{ minWidth: '75px' }}
>
{selectableQueryIndexes.map((qi) => (
<MenuItem key={`query-${qi}`} value={qi}>
#{qi + 1}
<MenuItem value={queryName}>{queryName}</MenuItem>
{selectableQueryNames.map((qn) => (
<MenuItem key={`query-${qn}`} value={qn}>
{qn}
</MenuItem>
))}
</TextField>
Expand All @@ -414,7 +447,7 @@ function QuerySettingsInput({
<MenuItem value="fixed">Fixed</MenuItem>
</TextField>
<OptionsColorPicker
label={`Query n°${queryIndex + 1}`}
label={queryName}
color={colorValue || DEFAULT_COLOR_VALUE}
onColorChange={onColorValueChange}
/>
Expand Down Expand Up @@ -449,7 +482,7 @@ function QuerySettingsInput({
{/* Area Opacity section */}
{areaOpacity !== undefined && (
<SettingsSection label="Opacity" onRemove={onRemoveAreaOpacity}>
{/* 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. */}
<Box />
<Slider
value={areaOpacity}
Expand Down Expand Up @@ -506,7 +539,7 @@ function QuerySettingsInput({
</Stack>
{/* Delete Button for this query settings */}
<Box sx={{ display: 'flex', alignItems: 'center', justifyContent: 'center' }}>
<IconButton aria-label={`delete settings for query n°${queryIndex + 1}`} onClick={onDelete}>
<IconButton aria-label={`delete settings for query '${queryName}'`} onClick={onDelete}>
<DeleteIcon />
</IconButton>
</Box>
Expand Down
Loading
Loading