From ebaccc781b62ac664b010ec2f7c3e91c337891c7 Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Wed, 26 Nov 2025 08:30:38 -0800 Subject: [PATCH 01/14] Suggestions: Update all suggestions suppliers to be functions (#113986) * Suggestions: Convert panels to use function supplier * rework deaggregation * BarGauge * cleanup and make consistent the deaggregation in suggestions * Candlestick * Implement timeseries and clean up some things that can already be deleted * spotted some typos in self-review * restore PanelDataSummary deprecated fields, we wont delete till Grafana 13 * change deprecation message * remove some unused imports * run prettier * update radialbar defaults logic * update tests and logic to DRY up the reduceOptions a bit and more thoroughly test the output * Trend: Improve suggestions * updates from review * add unique DataFrameType list to PanelDataSummary * add histogram suggestions * rework panelDataSummary to be a class, change some things * further boil down PanelDataSummary * Improve FlameGgraph suggestions * geomap and other defaults * reorder the single frame with string and number test --- .../grafana-data/src/panel/PanelPlugin.ts | 5 + .../panel/suggestions/getPanelDataSummary.ts | 162 +++++++--- .../transformers/calculateField.ts | 6 +- .../grafana-data/src/types/suggestions.ts | 2 + public/app/features/geo/utils/location.ts | 10 + .../panel/components/PanelDataErrorView.tsx | 7 +- .../suggestions/getAllSuggestions.test.ts | 255 ++++++++++----- .../panel/suggestions/getAllSuggestions.ts | 33 +- .../app/features/panel/suggestions/utils.ts | 45 ++- public/app/plugins/panel/barchart/module.tsx | 4 +- .../app/plugins/panel/barchart/suggestions.ts | 161 +++++----- public/app/plugins/panel/bargauge/module.tsx | 4 +- .../app/plugins/panel/bargauge/suggestions.ts | 157 +++------ .../app/plugins/panel/candlestick/module.tsx | 4 +- .../plugins/panel/candlestick/suggestions.ts | 71 ++--- .../panel/flamegraph/FlameGraphPanel.tsx | 4 +- .../app/plugins/panel/flamegraph/module.tsx | 23 +- .../plugins/panel/flamegraph/suggestions.ts | 31 -- public/app/plugins/panel/flamegraph/types.ts | 3 + public/app/plugins/panel/gauge/module.tsx | 4 +- public/app/plugins/panel/gauge/suggestions.ts | 136 ++++---- public/app/plugins/panel/geomap/module.tsx | 4 +- .../app/plugins/panel/geomap/suggestions.ts | 46 +++ public/app/plugins/panel/heatmap/module.tsx | 4 +- .../plugins/panel/heatmap/suggestions.test.ts | 226 +++++++++++++ .../app/plugins/panel/heatmap/suggestions.ts | 102 +++--- public/app/plugins/panel/histogram/module.tsx | 15 + public/app/plugins/panel/logs/module.tsx | 8 +- public/app/plugins/panel/logs/suggestions.ts | 33 -- public/app/plugins/panel/nodeGraph/module.tsx | 4 +- .../plugins/panel/nodeGraph/suggestions.ts | 119 ++++--- public/app/plugins/panel/piechart/module.tsx | 4 +- .../app/plugins/panel/piechart/suggestions.ts | 133 ++++---- public/app/plugins/panel/radialbar/module.tsx | 4 +- .../panel/radialbar/suggestions.test.ts | 18 +- .../plugins/panel/radialbar/suggestions.ts | 112 +++---- public/app/plugins/panel/stat/module.tsx | 4 +- public/app/plugins/panel/stat/suggestions.ts | 127 ++++---- .../plugins/panel/state-timeline/module.tsx | 2 +- .../plugins/panel/status-history/module.tsx | 42 ++- .../panel/status-history/suggestions.ts | 56 ---- public/app/plugins/panel/table/module.tsx | 4 +- public/app/plugins/panel/table/suggestions.ts | 61 ++-- .../app/plugins/panel/timeseries/module.tsx | 4 +- .../plugins/panel/timeseries/suggestions.ts | 297 +++++++----------- public/app/plugins/panel/traces/module.tsx | 4 +- .../app/plugins/panel/traces/suggestions.ts | 29 -- public/app/plugins/panel/trend/TrendPanel.tsx | 50 +-- public/app/plugins/panel/trend/module.tsx | 51 ++- public/app/plugins/panel/trend/suggestions.ts | 43 --- public/app/plugins/panel/trend/utils.ts | 48 +++ public/app/types/suggestions.ts | 34 -- public/locales/en-US/grafana.json | 33 ++ 53 files changed, 1573 insertions(+), 1275 deletions(-) delete mode 100644 public/app/plugins/panel/flamegraph/suggestions.ts create mode 100644 public/app/plugins/panel/flamegraph/types.ts create mode 100644 public/app/plugins/panel/geomap/suggestions.ts create mode 100644 public/app/plugins/panel/heatmap/suggestions.test.ts delete mode 100644 public/app/plugins/panel/logs/suggestions.ts delete mode 100644 public/app/plugins/panel/status-history/suggestions.ts delete mode 100644 public/app/plugins/panel/traces/suggestions.ts delete mode 100644 public/app/plugins/panel/trend/suggestions.ts create mode 100644 public/app/plugins/panel/trend/utils.ts delete mode 100644 public/app/types/suggestions.ts diff --git a/packages/grafana-data/src/panel/PanelPlugin.ts b/packages/grafana-data/src/panel/PanelPlugin.ts index fdae270fd48..7b766227ba1 100644 --- a/packages/grafana-data/src/panel/PanelPlugin.ts +++ b/packages/grafana-data/src/panel/PanelPlugin.ts @@ -381,6 +381,11 @@ export class PanelPlugin< const appender = builder.getListAppender({ pluginId: this.meta.id, name: this.meta.name, + options: {}, + fieldConfig: { + defaults: {}, + overrides: [], + }, }); const result = supplier(builder.dataSummary); diff --git a/packages/grafana-data/src/panel/suggestions/getPanelDataSummary.ts b/packages/grafana-data/src/panel/suggestions/getPanelDataSummary.ts index ea97e1705d7..021d1eb48c6 100644 --- a/packages/grafana-data/src/panel/suggestions/getPanelDataSummary.ts +++ b/packages/grafana-data/src/panel/suggestions/getPanelDataSummary.ts @@ -1,19 +1,27 @@ import { PreferredVisualisationType } from '../../types/data'; import { DataFrame, FieldType } from '../../types/dataFrame'; +import { DataFrameType } from '../../types/dataFrameTypes'; -/** - * @alpha - */ export interface PanelDataSummary { hasData?: boolean; rowCountTotal: number; + /** max number of rows in any given dataframe in the panel data */ rowCountMax: number; frameCount: number; fieldCount: number; + /** max number of fields in any given dataframe in the panel data */ + fieldCountMax: number; + /** given a field type, return the number of fields across all dataframes which match this type */ fieldCountByType: (type: FieldType) => number; + /** returns true if any fields in any frames match the field type */ hasFieldType: (type: FieldType) => boolean; - /** The first frame that set's this value */ - preferredVisualisationType?: PreferredVisualisationType; + /* returns true if any of the frames in this panel data summary have the type */ + hasDataFrameType: (type: DataFrameType) => boolean; + /* returns true if any of the frames in this panel data summary have the type */ + hasPreferredVisualisationType: (type: PreferredVisualisationType) => boolean; + + /** pass along a reference to the DataFrame array in case it's needed by the plugin */ + rawFrames?: DataFrame[]; /* --- DEPRECATED FIELDS BELOW --- */ /** @deprecated use PanelDataSummary.fieldCountByType(FieldType.number) */ @@ -23,60 +31,114 @@ export interface PanelDataSummary { /** @deprecated use PanelDataSummary.fieldCountByType(FieldType.string) */ stringFieldCount: number; /** @deprecated use PanelDataSummary.hasFieldType(FieldType.number) */ - hasNumberField?: boolean; - /** @deprecated use PanelDataSummary.hasFieldType(FieldType.time) */ hasTimeField?: boolean; + /** @deprecated use PanelDataSummary.hasFieldType(FieldType.time) */ + hasNumberField?: boolean; /** @deprecated use PanelDataSummary.hasFieldType(FieldType.string) */ hasStringField?: boolean; } +/** + * @alpha + */ +class PanelDataSummaryImpl implements PanelDataSummary { + public rowCountTotal = 0; + /** max number of rows in any single dataframe in the panel data */ + public rowCountMax = 0; + public fieldCount = 0; + /** max number of fields in any single dataframe in the panel data */ + public fieldCountMax = 0; + + private countByType: Partial> = {}; + private preferredVisualisationTypes: Set = new Set(); + private dataFrameTypes: Set = new Set(); + + public get hasData(): boolean { + return this.rowCountTotal > 0; + } + + public get frameCount(): number { + return this.rawFrames?.length ?? 0; + } + + constructor(public rawFrames?: DataFrame[]) { + this._processFrames(); + } + + private _processFrames() { + for (const frame of this.rawFrames ?? []) { + this.rowCountTotal += frame.length; + + if (frame.meta?.preferredVisualisationType) { + this.preferredVisualisationTypes.add(frame.meta.preferredVisualisationType); + } + if (frame.meta?.type) { + this.dataFrameTypes.add(frame.meta.type); + } + + for (const field of frame.fields) { + this.fieldCount++; + this.countByType[field.type] = (this.countByType[field.type] || 0) + 1; + } + + if (frame.length > this.rowCountMax) { + this.rowCountMax = frame.length; + } + if (frame.fields.length > this.fieldCountMax) { + this.fieldCountMax = frame.fields.length; + } + } + } + + public fieldCountByType(type: FieldType): number { + return this.countByType[type] ?? 0; + } + + public hasFieldType(type: FieldType): boolean { + return this.fieldCountByType(type) > 0; + } + + public hasPreferredVisualisationType(type: PreferredVisualisationType): boolean { + return this.preferredVisualisationTypes.has(type); + } + + public hasDataFrameType(type: DataFrameType): boolean { + return this.dataFrameTypes.has(type); + } + + /**** DEPRECATED ****/ + /** @deprecated use PanelDataSummary.fieldCountByType(FieldType.number) */ + public get numberFieldCount(): number { + return this.fieldCountByType(FieldType.number); + } + /** @deprecated use PanelDataSummary.fieldCountByType(FieldType.time) */ + public get timeFieldCount(): number { + return this.fieldCountByType(FieldType.time); + } + /** @deprecated use PanelDataSummary.fieldCountByType(FieldType.string) */ + public get stringFieldCount() { + return this.fieldCountByType(FieldType.string); + } + /** @deprecated use PanelDataSummary.hasFieldType(FieldType.number) */ + public get hasTimeField() { + return this.fieldCountByType(FieldType.time) > 0; + } + /** @deprecated use PanelDataSummary.hasFieldType(FieldType.time) */ + public get hasNumberField() { + return this.fieldCountByType(FieldType.number) > 0; + } + /** @deprecated use PanelDataSummary.hasFieldType(FieldType.string) */ + public get hasStringField() { + return this.fieldCountByType(FieldType.string) > 0; + } +} + /** * @alpha * given a list of dataframes, summarize attributes of those frames for features like suggestions. * @param frames - dataframes to summarize * @returns summary of the dataframes */ -export function getPanelDataSummary(frames: DataFrame[] = []): PanelDataSummary { - let rowCountTotal = 0; - let rowCountMax = 0; - let fieldCount = 0; - const countByType: Partial> = {}; - let preferredVisualisationType: PreferredVisualisationType | undefined; - - for (const frame of frames) { - rowCountTotal += frame.length; - - if (frame.meta?.preferredVisualisationType) { - preferredVisualisationType = frame.meta.preferredVisualisationType; - } - - for (const field of frame.fields) { - fieldCount++; - countByType[field.type] = (countByType[field.type] || 0) + 1; - } - - if (frame.length > rowCountMax) { - rowCountMax = frame.length; - } - } - - const fieldCountByType = (f: FieldType) => countByType[f] ?? 0; - - return { - rowCountTotal, - rowCountMax, - fieldCount, - preferredVisualisationType, - frameCount: frames.length, - hasData: rowCountTotal > 0, - hasFieldType: (f: FieldType) => fieldCountByType(f) > 0, - fieldCountByType, - // deprecated - numberFieldCount: fieldCountByType(FieldType.number), - timeFieldCount: fieldCountByType(FieldType.time), - stringFieldCount: fieldCountByType(FieldType.string), - hasTimeField: fieldCountByType(FieldType.time) > 0, - hasNumberField: fieldCountByType(FieldType.number) > 0, - hasStringField: fieldCountByType(FieldType.string) > 0, - }; +export function getPanelDataSummary(frames?: DataFrame[]): PanelDataSummary { + return new PanelDataSummaryImpl(frames); } diff --git a/packages/grafana-data/src/transformations/transformers/calculateField.ts b/packages/grafana-data/src/transformations/transformers/calculateField.ts index 733155a3f10..f1023b49502 100644 --- a/packages/grafana-data/src/transformations/transformers/calculateField.ts +++ b/packages/grafana-data/src/transformations/transformers/calculateField.ts @@ -72,7 +72,7 @@ interface IndexOptions { asPercentile: boolean; } -const defaultReduceOptions: ReduceOptions = { +const defaultNumericVizOptions: ReduceOptions = { reducer: ReducerID.sum, }; @@ -149,10 +149,10 @@ export const calculateFieldTransformer: DataTransformerInfo) => void; + /** @deprecated this will no longer be supported in the new Suggestions UI. */ icon?: string; + /** @deprecated this will no longer be supported in the new Suggestions UI. */ imgSrc?: string; }; } diff --git a/public/app/features/geo/utils/location.ts b/public/app/features/geo/utils/location.ts index 96e92929cdd..90aaa0f9d1e 100644 --- a/public/app/features/geo/utils/location.ts +++ b/public/app/features/geo/utils/location.ts @@ -68,6 +68,16 @@ const defaultMatchers: LocationFieldMatchers = { geo: (frame: DataFrame) => frame.fields.find((f) => f.type === FieldType.geo), }; +/** + * suggestions needs to run sync, and we just want to use the default matchers in that situation. + */ +export function getDefaultLocationMatchers(): LocationFieldMatchers { + return { + ...defaultMatchers, + mode: FrameGeometrySourceMode.Auto, + }; +} + export async function getLocationMatchers(src?: FrameGeometrySource): Promise { const info: LocationFieldMatchers = { ...defaultMatchers, diff --git a/public/app/features/panel/components/PanelDataErrorView.tsx b/public/app/features/panel/components/PanelDataErrorView.tsx index 4c72a3c879f..93723b3bff1 100644 --- a/public/app/features/panel/components/PanelDataErrorView.tsx +++ b/public/app/features/panel/components/PanelDataErrorView.tsx @@ -2,6 +2,7 @@ import { css } from '@emotion/css'; import { CoreApp, + FieldType, getPanelDataSummary, GrafanaTheme2, PanelDataSummary, @@ -134,15 +135,15 @@ function getMessageFor( return fieldConfig?.defaults.noValue ?? t('panel.panel-data-error-view.no-value.default', 'No data'); } - if (needsStringField && !dataSummary.hasStringField) { + if (needsStringField && !dataSummary.hasFieldType(FieldType.string)) { return t('panel.panel-data-error-view.missing-value.string', 'Data is missing a string field'); } - if (needsNumberField && !dataSummary.hasNumberField) { + if (needsNumberField && !dataSummary.hasFieldType(FieldType.number)) { return t('panel.panel-data-error-view.missing-value.number', 'Data is missing a number field'); } - if (needsTimeField && !dataSummary.hasTimeField) { + if (needsTimeField && !dataSummary.hasFieldType(FieldType.time)) { return t('panel.panel-data-error-view.missing-value.time', 'Data is missing a time field'); } diff --git a/public/app/features/panel/suggestions/getAllSuggestions.test.ts b/public/app/features/panel/suggestions/getAllSuggestions.test.ts index e4a0c72c5d9..1cb10605ab0 100644 --- a/public/app/features/panel/suggestions/getAllSuggestions.test.ts +++ b/public/app/features/panel/suggestions/getAllSuggestions.test.ts @@ -5,12 +5,18 @@ import { LoadingState, PanelData, PanelPluginMeta, - toDataFrame, PanelPluginVisualizationSuggestion, + toDataFrame, } from '@grafana/data'; -import { GraphFieldConfig, ReduceDataOptions } from '@grafana/schema'; +import { + BarGaugeDisplayMode, + BigValueColorMode, + GraphFieldConfig, + ReduceDataOptions, + StackingMode, + VizOrientation, +} from '@grafana/schema'; import { config } from 'app/core/config'; -import { SuggestionName } from 'app/types/suggestions'; import { getAllSuggestions, panelsToCheckFirst } from './getAllSuggestions'; @@ -21,6 +27,8 @@ for (const pluginId of panelsToCheckFirst) { } as PanelPluginMeta; } +const SCALAR_PLUGINS = ['gauge', 'stat', 'bargauge', 'piechart', 'radialbar']; + config.panels['text'] = { id: 'text', name: 'Text', @@ -69,7 +77,10 @@ scenario('No series', (ctx) => { ctx.setData([]); it('should return correct suggestions', () => { - expect(ctx.names()).toEqual([SuggestionName.Table, SuggestionName.TextPanel]); + expect(ctx.suggestions).toEqual([ + expect.objectContaining({ pluginId: 'table' }), + expect.objectContaining({ pluginId: 'text' }), + ]); }); }); @@ -84,7 +95,7 @@ scenario('No rows', (ctx) => { ]); it('should return correct suggestions', () => { - expect(ctx.names()).toEqual([SuggestionName.Table]); + expect(ctx.suggestions).toEqual([expect.objectContaining({ pluginId: 'table' })]); }); }); @@ -100,33 +111,46 @@ scenario('Single frame with time and number field', (ctx) => { it('should return correct suggestions', () => { expect(ctx.suggestions).toEqual([ - expect.objectContaining({ name: SuggestionName.LineChart }), - expect.objectContaining({ name: SuggestionName.LineChartSmooth }), - expect.objectContaining({ name: SuggestionName.AreaChart }), - expect.objectContaining({ name: SuggestionName.LineChartGradientColorScheme }), - expect.objectContaining({ name: SuggestionName.BarChart }), - expect.objectContaining({ name: SuggestionName.BarChartGradientColorScheme }), - expect.objectContaining({ name: SuggestionName.Gauge }), - expect.objectContaining({ name: SuggestionName.GaugeNoThresholds }), - expect.objectContaining({ name: SuggestionName.Stat }), - expect.objectContaining({ name: SuggestionName.StatColoredBackground }), - expect.objectContaining({ name: SuggestionName.BarGaugeBasic }), - expect.objectContaining({ name: SuggestionName.BarGaugeLCD }), - expect.objectContaining({ name: SuggestionName.Table }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Line chart' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Line chart - smooth' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Area chart' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Bar chart' }), + expect.objectContaining({ pluginId: 'gauge' }), + expect.objectContaining({ pluginId: 'gauge', options: expect.objectContaining({ showThresholdMarkers: false }) }), + expect.objectContaining({ pluginId: 'stat' }), + expect.objectContaining({ + pluginId: 'stat', + options: expect.objectContaining({ colorMode: BigValueColorMode.Background }), + }), + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Basic }), + }), + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Lcd }), + }), + expect.objectContaining({ pluginId: 'table' }), expect.objectContaining({ pluginId: 'state-timeline' }), - expect.objectContaining({ name: SuggestionName.StatusHistory }), + expect.objectContaining({ pluginId: 'status-history' }), + expect.objectContaining({ pluginId: 'heatmap' }), + expect.objectContaining({ pluginId: 'histogram' }), ]); }); it('Bar chart suggestion should be using timeseries panel', () => { - expect(ctx.suggestions.find((x) => x.name === SuggestionName.BarChart)?.pluginId).toBe('timeseries'); + expect(ctx.suggestions.find((x) => x.name === 'Bar chart')?.pluginId).toBe('timeseries'); }); - it('Stat panels have reduce values disabled', () => { - for (const suggestion of ctx.suggestions) { - if (suggestion.options?.reduceOptions?.values) { - throw new Error(`Suggestion ${suggestion.name} reduce.values set to true when it should be false`); - } + it('Scalar panels should use calcs', () => { + for (const suggestion of ctx.suggestions.filter((s) => SCALAR_PLUGINS.includes(s.pluginId))) { + expect(suggestion).toEqual( + expect.objectContaining({ + options: expect.objectContaining({ + reduceOptions: expect.objectContaining({ values: false, calcs: ['lastNotNull'] }), + }), + }) + ); } }); }); @@ -144,31 +168,46 @@ scenario('Single frame with time 2 number fields', (ctx) => { it('should return correct suggestions', () => { expect(ctx.suggestions).toEqual([ - expect.objectContaining({ name: SuggestionName.LineChart }), - expect.objectContaining({ name: SuggestionName.LineChartSmooth }), - expect.objectContaining({ name: SuggestionName.AreaChartStacked }), - expect.objectContaining({ name: SuggestionName.AreaChartStackedPercent }), - expect.objectContaining({ name: SuggestionName.BarChartStacked }), - expect.objectContaining({ name: SuggestionName.BarChartStackedPercent }), - expect.objectContaining({ name: SuggestionName.Gauge }), - expect.objectContaining({ name: SuggestionName.GaugeNoThresholds }), - expect.objectContaining({ name: SuggestionName.Stat }), - expect.objectContaining({ name: SuggestionName.StatColoredBackground }), - expect.objectContaining({ name: SuggestionName.PieChart }), - expect.objectContaining({ name: SuggestionName.PieChartDonut }), - expect.objectContaining({ name: SuggestionName.BarGaugeBasic }), - expect.objectContaining({ name: SuggestionName.BarGaugeLCD }), - expect.objectContaining({ name: SuggestionName.Table }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Line chart' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Line chart - smooth' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Area chart - stacked' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Area chart - stacked by percentage' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Bar chart - stacked' }), + expect.objectContaining({ pluginId: 'timeseries', name: 'Bar chart - stacked by percentage' }), + expect.objectContaining({ pluginId: 'gauge' }), + expect.objectContaining({ pluginId: 'gauge', options: expect.objectContaining({ showThresholdMarkers: false }) }), + expect.objectContaining({ pluginId: 'stat' }), + expect.objectContaining({ + pluginId: 'stat', + options: expect.objectContaining({ colorMode: BigValueColorMode.Background }), + }), + expect.objectContaining({ pluginId: 'piechart' }), + expect.objectContaining({ pluginId: 'piechart', options: expect.objectContaining({ pieType: 'donut' }) }), + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Basic }), + }), + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Lcd }), + }), + expect.objectContaining({ pluginId: 'table' }), expect.objectContaining({ pluginId: 'state-timeline' }), - expect.objectContaining({ name: SuggestionName.StatusHistory }), + expect.objectContaining({ pluginId: 'status-history' }), + expect.objectContaining({ pluginId: 'heatmap' }), + expect.objectContaining({ pluginId: 'histogram' }), ]); }); - it('Stat panels have reduceOptions.values disabled', () => { - for (const suggestion of ctx.suggestions) { - if (suggestion.options?.reduceOptions?.values) { - throw new Error(`Suggestion ${suggestion.name} reduce.values set to true when it should be false`); - } + it('Scalar panels should use calcs', () => { + for (const suggestion of ctx.suggestions.filter((s) => SCALAR_PLUGINS.includes(s.pluginId))) { + expect(suggestion).toEqual( + expect.objectContaining({ + options: expect.objectContaining({ + reduceOptions: expect.objectContaining({ values: false, calcs: ['lastNotNull'] }), + }), + }) + ); } }); }); @@ -184,7 +223,7 @@ scenario('Single time series with 100 data points', (ctx) => { ]); it('should not suggest bar chart', () => { - expect(ctx.suggestions.find((x) => x.name === SuggestionName.BarChart)).toBe(undefined); + expect(ctx.suggestions.find((x) => x.name === 'Bar chart')).toBe(undefined); }); }); @@ -235,26 +274,42 @@ scenario('Single frame with string and number field', (ctx) => { ]); it('should return correct suggestions', () => { - expect(ctx.names()).toEqual([ - SuggestionName.BarChart, - SuggestionName.BarChartHorizontal, - SuggestionName.Gauge, - SuggestionName.GaugeNoThresholds, - SuggestionName.Stat, - SuggestionName.StatColoredBackground, - SuggestionName.PieChart, - SuggestionName.PieChartDonut, - SuggestionName.BarGaugeBasic, - SuggestionName.BarGaugeLCD, - SuggestionName.Table, + expect(ctx.suggestions).toEqual([ + expect.objectContaining({ pluginId: 'piechart' }), + expect.objectContaining({ pluginId: 'piechart', options: expect.objectContaining({ pieType: 'donut' }) }), + expect.objectContaining({ pluginId: 'barchart' }), + expect.objectContaining({ + pluginId: 'barchart', + options: expect.objectContaining({ orientation: VizOrientation.Horizontal }), + }), + expect.objectContaining({ pluginId: 'gauge' }), + expect.objectContaining({ pluginId: 'gauge', options: expect.objectContaining({ showThresholdMarkers: false }) }), + expect.objectContaining({ pluginId: 'stat' }), + expect.objectContaining({ + pluginId: 'stat', + options: expect.objectContaining({ colorMode: BigValueColorMode.Background }), + }), + + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Basic }), + }), + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Lcd }), + }), + expect.objectContaining({ pluginId: 'table' }), + expect.objectContaining({ pluginId: 'histogram' }), ]); }); - it('Stat/Gauge/BarGauge/PieChart panels to have reduceOptions.values enabled', () => { - for (const suggestion of ctx.suggestions) { - if (suggestion.options?.reduceOptions && !suggestion.options?.reduceOptions?.values) { - throw new Error(`Suggestion ${suggestion.name} reduce.values set to false when it should be true`); - } + it('Scalar panels should contain raw values', () => { + for (const suggestion of ctx.suggestions.filter((s) => SCALAR_PLUGINS.includes(s.pluginId))) { + expect(suggestion).toEqual( + expect.objectContaining({ + options: expect.objectContaining({ reduceOptions: expect.objectContaining({ values: true, calcs: [] }) }), + }) + ); } }); }); @@ -271,22 +326,48 @@ scenario('Single frame with string and 2 number field', (ctx) => { ]); it('should return correct suggestions', () => { - expect(ctx.names()).toEqual([ - SuggestionName.BarChart, - SuggestionName.BarChartStacked, - SuggestionName.BarChartStackedPercent, - SuggestionName.BarChartHorizontal, - SuggestionName.BarChartHorizontalStacked, - SuggestionName.BarChartHorizontalStackedPercent, - SuggestionName.Gauge, - SuggestionName.GaugeNoThresholds, - SuggestionName.Stat, - SuggestionName.StatColoredBackground, - SuggestionName.PieChart, - SuggestionName.PieChartDonut, - SuggestionName.BarGaugeBasic, - SuggestionName.BarGaugeLCD, - SuggestionName.Table, + expect(ctx.suggestions).toEqual([ + expect.objectContaining({ pluginId: 'barchart' }), + expect.objectContaining({ + pluginId: 'barchart', + options: expect.objectContaining({ stacking: StackingMode.Normal }), + }), + expect.objectContaining({ + pluginId: 'barchart', + options: expect.objectContaining({ stacking: StackingMode.Percent }), + }), + + expect.objectContaining({ + pluginId: 'barchart', + options: expect.objectContaining({ orientation: VizOrientation.Horizontal }), + }), + expect.objectContaining({ + pluginId: 'barchart', + options: expect.objectContaining({ orientation: VizOrientation.Horizontal, stacking: StackingMode.Normal }), + }), + expect.objectContaining({ + pluginId: 'barchart', + options: expect.objectContaining({ orientation: VizOrientation.Horizontal, stacking: StackingMode.Percent }), + }), + expect.objectContaining({ pluginId: 'gauge' }), + expect.objectContaining({ pluginId: 'gauge', options: expect.objectContaining({ showThresholdMarkers: false }) }), + expect.objectContaining({ pluginId: 'stat' }), + expect.objectContaining({ + pluginId: 'stat', + options: expect.objectContaining({ colorMode: BigValueColorMode.Background }), + }), + expect.objectContaining({ pluginId: 'piechart' }), + expect.objectContaining({ pluginId: 'piechart', options: expect.objectContaining({ pieType: 'donut' }) }), + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Basic }), + }), + expect.objectContaining({ + pluginId: 'bargauge', + options: expect.objectContaining({ displayMode: BarGaugeDisplayMode.Lcd }), + }), + expect.objectContaining({ pluginId: 'table' }), + expect.objectContaining({ pluginId: 'histogram' }), ]); }); }); @@ -299,11 +380,14 @@ scenario('Single frame with only string field', (ctx) => { ]); it('should return correct suggestions', () => { - expect(ctx.names()).toEqual([SuggestionName.Stat, SuggestionName.Table]); + expect(ctx.suggestions).toEqual([ + expect.objectContaining({ pluginId: 'stat' }), + expect.objectContaining({ pluginId: 'table' }), + ]); }); it('Stat panels have reduceOptions.fields set to show all fields', () => { - for (const suggestion of ctx.suggestions) { + for (const suggestion of ctx.suggestions.filter((s) => s.pluginId === 'stat')) { if (suggestion.options?.reduceOptions) { expect(suggestion.options.reduceOptions.fields).toBe('/.*/'); } @@ -333,7 +417,10 @@ scenario('Given default loki logs data', (ctx) => { ]); it('should return correct suggestions', () => { - expect(ctx.names()).toEqual([SuggestionName.Logs, SuggestionName.Table]); + expect(ctx.suggestions).toEqual([ + expect.objectContaining({ pluginId: 'logs' }), + expect.objectContaining({ pluginId: 'table' }), + ]); }); }); @@ -356,7 +443,7 @@ scenario('Given a preferredVisualisationType', (ctx) => { ]); it('should return the preferred visualization first', () => { - expect(ctx.names()[0]).toEqual(SuggestionName.Table); + expect(ctx.suggestions[0]).toEqual(expect.objectContaining({ pluginId: 'table' })); }); }); diff --git a/public/app/features/panel/suggestions/getAllSuggestions.ts b/public/app/features/panel/suggestions/getAllSuggestions.ts index ff6fbef50e8..a7c9764d2a0 100644 --- a/public/app/features/panel/suggestions/getAllSuggestions.ts +++ b/public/app/features/panel/suggestions/getAllSuggestions.ts @@ -4,6 +4,7 @@ import { VisualizationSuggestionsBuilder, PanelModel, VisualizationSuggestionScore, + PreferredVisualisationType, } from '@grafana/data'; import { config } from '@grafana/runtime'; import { importPanelPlugin } from 'app/features/plugins/importPanelPlugin'; @@ -23,8 +24,26 @@ export const panelsToCheckFirst = [ 'flamegraph', 'traces', 'nodeGraph', + 'heatmap', + 'histogram', + 'geomap', ]; +/** + * some of the PreferredVisualisationTypes do not match the panel plugin ids, so we have to map them. d'oh. + */ +const PLUGIN_ID_TO_PREFERRED_VIZ_TYPE: Record = { + traces: 'trace', + timeseries: 'graph', + table: 'table', + logs: 'logs', + nodeGraph: 'nodeGraph', + flamegraph: 'flamegraph', +}; +const mapPreferredVisualisationTypeToPlugin = (type: string): PreferredVisualisationType | undefined => { + return PLUGIN_ID_TO_PREFERRED_VIZ_TYPE[type]; +}; + export async function getAllSuggestions( data?: PanelData, panel?: PanelModel @@ -61,13 +80,13 @@ export async function getAllSuggestions( } return list.sort((a, b) => { - if (builder.dataSummary.preferredVisualisationType) { - if (a.pluginId === builder.dataSummary.preferredVisualisationType) { - return -1; - } - if (b.pluginId === builder.dataSummary.preferredVisualisationType) { - return 1; - } + const mappedA = mapPreferredVisualisationTypeToPlugin(a.pluginId); + if (mappedA && builder.dataSummary.hasPreferredVisualisationType(mappedA)) { + return -1; + } + const mappedB = mapPreferredVisualisationTypeToPlugin(a.pluginId); + if (mappedB && builder.dataSummary.hasPreferredVisualisationType(mappedB)) { + return 1; } return (b.score ?? VisualizationSuggestionScore.OK) - (a.score ?? VisualizationSuggestionScore.OK); }); diff --git a/public/app/features/panel/suggestions/utils.ts b/public/app/features/panel/suggestions/utils.ts index ca5cc74bd77..07820587be5 100644 --- a/public/app/features/panel/suggestions/utils.ts +++ b/public/app/features/panel/suggestions/utils.ts @@ -1,4 +1,11 @@ -import { PanelData, PanelDataSummary } from '@grafana/data'; +import { + DataFrameType, + PanelData, + PanelDataSummary, + VisualizationSuggestion, + VisualizationSuggestionScore, +} from '@grafana/data'; +import { ReduceDataOptions } from '@grafana/schema'; /** * @internal @@ -10,6 +17,42 @@ export function showDefaultSuggestion(fn: (panelDataSummary: PanelDataSummary) = return (panelDataSummary: PanelDataSummary) => (fn(panelDataSummary) ? [{}] : undefined); } +/** + * @internal + * for panel plugins which render "scalar" data (stat, gauge, etc), this helper provides default reduce options + * depending on whether deaggregation is likely needed. + * @param suggestion the suggestion to modify + * @param panelDataSummary the panel data summary to use for scoring + * @param shouldUseRawValues if true, reduceOptions will be set to use raw values, + * otherwise a calcs will be used with the default value of `lastNotNull`. + */ +export function defaultNumericVizOptions( + suggestion: VisualizationSuggestion<{ reduceOptions?: ReduceDataOptions }>, + panelDataSummary: PanelDataSummary, + shouldUseRawValues: boolean +): VisualizationSuggestion { + suggestion.score = + (suggestion.score ?? + (panelDataSummary.hasDataFrameType(DataFrameType.NumericLong) || + panelDataSummary.hasDataFrameType(DataFrameType.NumericWide) || + panelDataSummary.hasDataFrameType(DataFrameType.NumericMulti))) + ? VisualizationSuggestionScore.Good + : VisualizationSuggestionScore.OK; + suggestion.options = suggestion.options ?? {}; + suggestion.options.reduceOptions = + suggestion.options.reduceOptions ?? + (shouldUseRawValues + ? { + values: true, + calcs: [], + } + : { + values: false, + calcs: ['lastNotNull'], + }); + return suggestion; +} + /** * @internal * Checks if the panel has data diff --git a/public/app/plugins/panel/barchart/module.tsx b/public/app/plugins/panel/barchart/module.tsx index d4284656595..28ba77d5476 100644 --- a/public/app/plugins/panel/barchart/module.tsx +++ b/public/app/plugins/panel/barchart/module.tsx @@ -18,7 +18,7 @@ import { BarChartPanel } from './BarChartPanel'; import { TickSpacingEditor } from './TickSpacingEditor'; import { changeToBarChartPanelMigrationHandler } from './migrations'; import { FieldConfig, Options, defaultFieldConfig, defaultOptions } from './panelcfg.gen'; -import { BarChartSuggestionsSupplier } from './suggestions'; +import { barchartSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(BarChartPanel) .setPanelChangeHandler(changeToBarChartPanelMigrationHandler) @@ -257,7 +257,7 @@ export const plugin = new PanelPlugin(BarChartPanel) commonOptionsBuilder.addLegendOptions(builder); commonOptionsBuilder.addTextSizeOptions(builder, { withValue: true }); }) - .setSuggestionsSupplier(new BarChartSuggestionsSupplier()); + .setSuggestionsSupplier(barchartSuggestionsSupplier); function countNumberFields(data?: DataFrame[]): number { let count = 0; diff --git a/public/app/plugins/panel/barchart/suggestions.ts b/public/app/plugins/panel/barchart/suggestions.ts index f9428d6e709..56918e4b346 100644 --- a/public/app/plugins/panel/barchart/suggestions.ts +++ b/public/app/plugins/panel/barchart/suggestions.ts @@ -1,99 +1,112 @@ -import { VisualizationSuggestionsBuilder, VizOrientation } from '@grafana/data'; +import { defaultsDeep } from 'lodash'; + +import { FieldType, VisualizationSuggestion, VisualizationSuggestionsSupplierFn, VizOrientation } from '@grafana/data'; +import { t } from '@grafana/i18n'; import { LegendDisplayMode, StackingMode, VisibilityMode } from '@grafana/schema'; -import { SuggestionName } from 'app/types/suggestions'; import { FieldConfig, Options } from './panelcfg.gen'; -export class BarChartSuggestionsSupplier { - getListWithDefaults(builder: VisualizationSuggestionsBuilder) { - return builder.getListAppender({ - name: SuggestionName.BarChart, - pluginId: 'barchart', - options: { - showValue: VisibilityMode.Never, - legend: { - calcs: [], - displayMode: LegendDisplayMode.List, - showLegend: true, - placement: 'right', - }, +const withDefaults = (suggestion: VisualizationSuggestion) => + defaultsDeep(suggestion, { + options: { + showValue: VisibilityMode.Never, + legend: { + calcs: [], + displayMode: LegendDisplayMode.List, + showLegend: true, + placement: 'right', }, - fieldConfig: { - defaults: { - unit: 'short', - custom: {}, - }, - overrides: [], + }, + fieldConfig: { + defaults: { + unit: 'short', + custom: {}, }, - cardOptions: { - previewModifier: (s) => { - s.options!.barWidth = 0.8; - }, + overrides: [], + }, + cardOptions: { + previewModifier: (s) => { + s.options!.barWidth = 0.8; + s.fieldConfig!.defaults!.custom!.hideFrom = { tooltip: false, legend: true, viz: false }; // hide legend in preview }, - }); + }, + } satisfies VisualizationSuggestion); + +export const barchartSuggestionsSupplier: VisualizationSuggestionsSupplierFn = (dataSummary) => { + if (dataSummary.frameCount !== 1) { + return; } - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const list = this.getListWithDefaults(builder); - const { dataSummary } = builder; + if (!dataSummary.hasFieldType(FieldType.number) || !dataSummary.hasFieldType(FieldType.string)) { + return; + } - if (dataSummary.frameCount !== 1) { - return; - } + // if you have this many rows barchart might not be a good fit + if (dataSummary.rowCountTotal > 50) { + return; + } - if (!dataSummary.hasNumberField || !dataSummary.hasStringField) { - return; - } + const result: Array> = [ + { + name: t('barchart.suggestions.vertical', 'Bar chart'), + }, + ]; - // if you have this many rows barchart might not be a good fit - if (dataSummary.rowCountTotal > 50) { - return; - } - - // Vertical bars - list.append({ - name: SuggestionName.BarChart, - }); - - if (dataSummary.numberFieldCount > 1) { - list.append({ - name: SuggestionName.BarChartStacked, + if (dataSummary.fieldCountByType(FieldType.number) > 1) { + result.push( + { + name: t('barchart.suggestions.vert-stacked', 'Bar chart - stacked'), options: { stacking: StackingMode.Normal, }, - }); - list.append({ - name: SuggestionName.BarChartStackedPercent, + }, + { + name: t('barchart.suggestions.vert-stacked-percent', 'Bar chart - stacked by percentage'), options: { stacking: StackingMode.Percent, }, - }); - } - - // horizontal bars - list.append({ - name: SuggestionName.BarChartHorizontal, - options: { - orientation: VizOrientation.Horizontal, - }, - }); - - if (dataSummary.numberFieldCount > 1) { - list.append({ - name: SuggestionName.BarChartHorizontalStacked, - options: { - stacking: StackingMode.Normal, - orientation: VizOrientation.Horizontal, + fieldConfig: { + overrides: [], + defaults: { + unit: 'percentunit', + }, }, - }); + } + ); + } - list.append({ - name: SuggestionName.BarChartHorizontalStackedPercent, + // horizontal bars + result.push({ + name: t('barchart.suggestions.horizontal', 'Horizontal bar chart'), + options: { + orientation: VizOrientation.Horizontal, + }, + }); + + if (dataSummary.fieldCountByType(FieldType.number) > 1) { + result.push( + { + name: t('barchart.suggestions.hz-stacked', 'Horizontal bar chart - stacked'), + options: { + orientation: VizOrientation.Horizontal, + stacking: StackingMode.Normal, + }, + }, + { + name: t('barchart.suggestions.hz-stacked-percent', 'Horizontal bar chart - stacked by percentage'), options: { orientation: VizOrientation.Horizontal, stacking: StackingMode.Percent, }, - }); - } + fieldConfig: { + overrides: [], + defaults: { + unit: 'percentunit', + }, + }, + } + ); } -} + + return result.map(withDefaults); +}; diff --git a/public/app/plugins/panel/bargauge/module.tsx b/public/app/plugins/panel/bargauge/module.tsx index 3e0262fb237..8e7fd5ccae6 100644 --- a/public/app/plugins/panel/bargauge/module.tsx +++ b/public/app/plugins/panel/bargauge/module.tsx @@ -8,7 +8,7 @@ import { addOrientationOption, addStandardDataReduceOptions } from '../stat/comm import { barGaugePanelMigrationHandler } from './BarGaugeMigrations'; import { BarGaugePanel } from './BarGaugePanel'; import { Options, defaultOptions } from './panelcfg.gen'; -import { BarGaugeSuggestionsSupplier } from './suggestions'; +import { barGaugeSugggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(BarGaugePanel) .useFieldConfig() @@ -151,4 +151,4 @@ export const plugin = new PanelPlugin(BarGaugePanel) }) .setPanelChangeHandler(sharedSingleStatPanelChangedHandler) .setMigrationHandler(barGaugePanelMigrationHandler) - .setSuggestionsSupplier(new BarGaugeSuggestionsSupplier()); + .setSuggestionsSupplier(barGaugeSugggestionsSupplier); diff --git a/public/app/plugins/panel/bargauge/suggestions.ts b/public/app/plugins/panel/bargauge/suggestions.ts index 61d48ce104a..dc8a451cac0 100644 --- a/public/app/plugins/panel/bargauge/suggestions.ts +++ b/public/app/plugins/panel/bargauge/suggestions.ts @@ -1,115 +1,60 @@ -import { FieldColorModeId, VisualizationSuggestionsBuilder, VizOrientation } from '@grafana/data'; +import { defaultsDeep } from 'lodash'; + +import { + FieldColorModeId, + FieldType, + VisualizationSuggestion, + VisualizationSuggestionsSupplierFn, + VizOrientation, +} from '@grafana/data'; +import { t } from '@grafana/i18n'; import { BarGaugeDisplayMode } from '@grafana/ui'; -import { SuggestionName } from 'app/types/suggestions'; +import { defaultNumericVizOptions } from 'app/features/panel/suggestions/utils'; import { Options } from './panelcfg.gen'; -export class BarGaugeSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary } = builder; - - if (!dataSummary.hasData || !dataSummary.hasNumberField) { - return; - } - - const list = builder.getListAppender({ - name: '', - pluginId: 'bargauge', - options: {}, - fieldConfig: { - defaults: { - custom: {}, +const withDefaults = (suggestion: VisualizationSuggestion): VisualizationSuggestion => + defaultsDeep(suggestion, { + options: { + displayMode: BarGaugeDisplayMode.Basic, + orientation: VizOrientation.Horizontal, + }, + fieldConfig: { + defaults: { + color: { + mode: FieldColorModeId.ContinuousGrYlRd, }, - overrides: [], }, - }); + overrides: [], + }, + }); - // This is probably not a good option for many numeric fields - if (dataSummary.numberFieldCount > 50) { - return; - } +const BAR_LIMIT = 30; - // To use show individual row values we also need a string field to give each value a name - if (dataSummary.hasStringField && dataSummary.frameCount === 1 && dataSummary.rowCountTotal < 30) { - list.append({ - name: SuggestionName.BarGaugeBasic, - options: { - reduceOptions: { - values: true, - calcs: [], - }, - displayMode: BarGaugeDisplayMode.Basic, - orientation: VizOrientation.Horizontal, - }, - fieldConfig: { - defaults: { - color: { - mode: FieldColorModeId.ContinuousGrYlRd, - }, - }, - overrides: [], - }, - }); - - list.append({ - name: SuggestionName.BarGaugeLCD, - options: { - reduceOptions: { - values: true, - calcs: [], - }, - displayMode: BarGaugeDisplayMode.Lcd, - orientation: VizOrientation.Horizontal, - }, - fieldConfig: { - defaults: { - color: { - mode: FieldColorModeId.ContinuousGrYlRd, - }, - }, - overrides: [], - }, - }); - } else { - list.append({ - name: SuggestionName.BarGaugeBasic, - options: { - displayMode: BarGaugeDisplayMode.Basic, - orientation: VizOrientation.Horizontal, - reduceOptions: { - values: false, - calcs: ['lastNotNull'], - }, - }, - fieldConfig: { - defaults: { - color: { - mode: FieldColorModeId.ContinuousGrYlRd, - }, - }, - overrides: [], - }, - }); - - list.append({ - name: SuggestionName.BarGaugeLCD, - options: { - displayMode: BarGaugeDisplayMode.Lcd, - orientation: VizOrientation.Horizontal, - reduceOptions: { - values: false, - calcs: ['lastNotNull'], - }, - }, - fieldConfig: { - defaults: { - color: { - mode: FieldColorModeId.ContinuousGrYlRd, - }, - }, - overrides: [], - }, - }); - } +export const barGaugeSugggestionsSupplier: VisualizationSuggestionsSupplierFn = (dataSummary) => { + if (!dataSummary.hasData || !dataSummary.hasFieldType(FieldType.number)) { + return; } -} + + // This is probably not a good option for many numeric fields + if (dataSummary.fieldCountByType(FieldType.number) > BAR_LIMIT) { + return; + } + + const suggestions: Array> = [ + { name: t('bargauge.suggestions.basic', 'Bar gauge') }, + { + name: t('bargauge.suggestions.lcd', 'Bar gauge - LCD'), + options: { + displayMode: BarGaugeDisplayMode.Lcd, + }, + }, + ]; + + const shouldUseRawValues = + dataSummary.hasFieldType(FieldType.string) && + dataSummary.frameCount === 1 && + dataSummary.rowCountTotal <= BAR_LIMIT; + + return suggestions.map((s) => defaultNumericVizOptions(withDefaults(s), dataSummary, shouldUseRawValues)); +}; diff --git a/public/app/plugins/panel/candlestick/module.tsx b/public/app/plugins/panel/candlestick/module.tsx index 8a57e4801c1..ae43037e1b1 100644 --- a/public/app/plugins/panel/candlestick/module.tsx +++ b/public/app/plugins/panel/candlestick/module.tsx @@ -8,7 +8,7 @@ import { defaultGraphConfig, getGraphFieldConfig } from '../timeseries/config'; import { CandlestickPanel } from './CandlestickPanel'; import { CandlestickData, getCandlestickFieldsInfo, FieldPickerInfo, prepareCandlestickFields } from './fields'; -import { CandlestickSuggestionsSupplier } from './suggestions'; +import { candlestickSuggestionSupplier } from './suggestions'; import { defaultCandlestickColors, defaultOptions, Options, VizDisplayMode, ColorStrategy, CandleStyle } from './types'; const numericFieldFilter = (f: Field) => f.type === FieldType.number; @@ -147,4 +147,4 @@ export const plugin = new PanelPlugin(CandlestickPane commonOptionsBuilder.addLegendOptions(builder); }) .setDataSupport({ annotations: true, alertStates: true }) - .setSuggestionsSupplier(new CandlestickSuggestionsSupplier()); + .setSuggestionsSupplier(candlestickSuggestionSupplier); diff --git a/public/app/plugins/panel/candlestick/suggestions.ts b/public/app/plugins/panel/candlestick/suggestions.ts index ce22cc276c5..81834af9db5 100644 --- a/public/app/plugins/panel/candlestick/suggestions.ts +++ b/public/app/plugins/panel/candlestick/suggestions.ts @@ -1,54 +1,29 @@ -import { VisualizationSuggestionsBuilder, VisualizationSuggestionScore } from '@grafana/data'; +import { FieldType, VisualizationSuggestionScore, VisualizationSuggestionsSupplierFn } from '@grafana/data'; import { config } from '@grafana/runtime'; -import { SuggestionName } from 'app/types/suggestions'; import { prepareCandlestickFields } from './fields'; import { defaultOptions, Options } from './types'; -export class CandlestickSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary } = builder; - - if ( - !builder.data?.series || - !dataSummary.hasData || - dataSummary.timeFieldCount < 1 || - dataSummary.numberFieldCount < 2 || - dataSummary.numberFieldCount > 10 - ) { - return; - } - - const info = prepareCandlestickFields(builder.data.series, defaultOptions, config.theme2); - if (!info) { - return; - } - - // Regular timeseries - if (info.open === info.high && info.open === info.low) { - return; - } - - const list = builder.getListAppender({ - name: '', - pluginId: 'candlestick', - options: {}, - fieldConfig: { - defaults: { - custom: {}, - }, - overrides: [], - }, - }); - - list.append({ - name: SuggestionName.Candlestick, - options: defaultOptions, - fieldConfig: { - defaults: {}, - overrides: [], - }, - score: info.autoOpenClose ? VisualizationSuggestionScore.Good : VisualizationSuggestionScore.Best, - }); +export const candlestickSuggestionSupplier: VisualizationSuggestionsSupplierFn = (dataSummary) => { + if ( + !dataSummary.rawFrames || + !dataSummary.hasData || + dataSummary.fieldCountByType(FieldType.time) < 1 || + dataSummary.fieldCountByType(FieldType.number) < 2 || + dataSummary.fieldCountByType(FieldType.number) > 10 + ) { + return; } -} + + const info = prepareCandlestickFields(dataSummary.rawFrames, defaultOptions, config.theme2); + if (!info) { + return; + } + + // Regular timeseries + if (info.open === info.high && info.open === info.low) { + return; + } + + return [{ score: info.autoOpenClose ? VisualizationSuggestionScore.Good : VisualizationSuggestionScore.Best }]; +}; diff --git a/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx b/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx index e9e8d9c8368..54a68a78af0 100644 --- a/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx +++ b/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx @@ -2,6 +2,8 @@ import { CoreApp, PanelProps } from '@grafana/data'; import { FlameGraph, checkFields, getMessageCheckFieldsResult } from '@grafana/flamegraph'; import { PanelDataErrorView, reportInteraction, config } from '@grafana/runtime'; +import { Options } from './types'; + function interaction(name: string, context: Record = {}) { reportInteraction(`grafana_flamegraph_${name}`, { app: CoreApp.Unknown, @@ -10,7 +12,7 @@ function interaction(name: string, context: Record = {} }); } -export const FlameGraphPanel = (props: PanelProps) => { +export const FlameGraphPanel = (props: PanelProps) => { const wrongFields = checkFields(props.data.series[0]); if (wrongFields) { return ( diff --git a/public/app/plugins/panel/flamegraph/module.tsx b/public/app/plugins/panel/flamegraph/module.tsx index 19fdd0afcfc..a80fcea3eee 100644 --- a/public/app/plugins/panel/flamegraph/module.tsx +++ b/public/app/plugins/panel/flamegraph/module.tsx @@ -1,12 +1,29 @@ import { FieldConfigProperty, PanelPlugin } from '@grafana/data'; +import { checkFields } from '@grafana/flamegraph'; import { FlameGraphPanel } from './FlameGraphPanel'; -import { FlameGraphSuggestionsSupplier } from './suggestions'; +import { Options } from './types'; const flamegraphConfigOptions = [FieldConfigProperty.Unit, FieldConfigProperty.Decimals]; -export const plugin = new PanelPlugin(FlameGraphPanel) - .setSuggestionsSupplier(new FlameGraphSuggestionsSupplier()) +export const plugin = new PanelPlugin(FlameGraphPanel) + // check that the first frame of the data has the required fields for a flamegraph + .setSuggestionsSupplier((ds) => { + if (!ds.rawFrames?.some((frame) => checkFields(frame) === undefined)) { + return; + } + + return [ + { + cardOptions: { + previewModifier: (s) => { + s.options = s.options || {}; + s.options.showFlameGraphOnly = true; + }, + }, + }, + ]; + }) .useFieldConfig({ disableStandardOptions: Object.values(FieldConfigProperty).filter((v) => !flamegraphConfigOptions.includes(v)), }); diff --git a/public/app/plugins/panel/flamegraph/suggestions.ts b/public/app/plugins/panel/flamegraph/suggestions.ts deleted file mode 100644 index eb997ee8078..00000000000 --- a/public/app/plugins/panel/flamegraph/suggestions.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { VisualizationSuggestionsBuilder } from '@grafana/data'; -import { checkFields } from '@grafana/flamegraph'; -import { SuggestionName } from 'app/types/suggestions'; - -export class FlameGraphSuggestionsSupplier { - getListWithDefaults(builder: VisualizationSuggestionsBuilder) { - return builder.getListAppender<{}, {}>({ - name: SuggestionName.FlameGraph, - pluginId: 'flamegraph', - }); - } - - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - if (!builder.data) { - return; - } - - const dataFrame = builder.data.series[0]; - if (!dataFrame) { - return; - } - const wrongFields = checkFields(dataFrame); - if (wrongFields) { - return; - } - - this.getListWithDefaults(builder).append({ - name: SuggestionName.FlameGraph, - }); - } -} diff --git a/public/app/plugins/panel/flamegraph/types.ts b/public/app/plugins/panel/flamegraph/types.ts new file mode 100644 index 00000000000..8e5544c54a2 --- /dev/null +++ b/public/app/plugins/panel/flamegraph/types.ts @@ -0,0 +1,3 @@ +export interface Options { + showFlameGraphOnly?: boolean; +} diff --git a/public/app/plugins/panel/gauge/module.tsx b/public/app/plugins/panel/gauge/module.tsx index 690698534a1..61ea853e943 100644 --- a/public/app/plugins/panel/gauge/module.tsx +++ b/public/app/plugins/panel/gauge/module.tsx @@ -8,7 +8,7 @@ import { addOrientationOption, addStandardDataReduceOptions } from '../stat/comm import { gaugePanelMigrationHandler, gaugePanelChangedHandler } from './GaugeMigrations'; import { GaugePanel } from './GaugePanel'; import { Options, defaultOptions } from './panelcfg.gen'; -import { GaugeSuggestionsSupplier } from './suggestions'; +import { gaugeSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(GaugePanel) .useFieldConfig({ @@ -88,5 +88,5 @@ export const plugin = new PanelPlugin(GaugePanel) commonOptionsBuilder.addTextSizeOptions(builder, { withTitle: true, withValue: true }); }) .setPanelChangeHandler(gaugePanelChangedHandler) - .setSuggestionsSupplier(new GaugeSuggestionsSupplier()) + .setSuggestionsSupplier(gaugeSuggestionsSupplier) .setMigrationHandler(gaugePanelMigrationHandler); diff --git a/public/app/plugins/panel/gauge/suggestions.ts b/public/app/plugins/panel/gauge/suggestions.ts index 5f5ebc8f40e..6695741b093 100644 --- a/public/app/plugins/panel/gauge/suggestions.ts +++ b/public/app/plugins/panel/gauge/suggestions.ts @@ -1,89 +1,63 @@ -import { ThresholdsMode, VisualizationSuggestionsBuilder } from '@grafana/data'; -import { GraphFieldConfig } from '@grafana/ui'; -import { SuggestionName } from 'app/types/suggestions'; +import { defaultsDeep } from 'lodash'; + +import { ThresholdsMode, FieldType, VisualizationSuggestion, VisualizationSuggestionsSupplierFn } from '@grafana/data'; +import { t } from '@grafana/i18n'; +import { defaultNumericVizOptions } from 'app/features/panel/suggestions/utils'; import { Options } from './panelcfg.gen'; -export class GaugeSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary } = builder; - - if (!dataSummary.hasData || !dataSummary.hasNumberField) { - return; - } - - // for many fields / series this is probably not a good fit - if (dataSummary.numberFieldCount >= 50) { - return; - } - - const list = builder.getListAppender({ - name: SuggestionName.Gauge, - pluginId: 'gauge', - options: {}, - fieldConfig: { - defaults: { - thresholds: { - steps: [ - { value: -Infinity, color: 'green' }, - { value: 70, color: 'orange' }, - { value: 85, color: 'red' }, - ], - mode: ThresholdsMode.Percentage, - }, - custom: {}, +const withDefaults = (suggestion: VisualizationSuggestion): VisualizationSuggestion => + defaultsDeep(suggestion, { + fieldConfig: { + defaults: { + thresholds: { + steps: [ + { value: -Infinity, color: 'green' }, + { value: 70, color: 'orange' }, + { value: 85, color: 'red' }, + ], + mode: ThresholdsMode.Percentage, }, - overrides: [], + custom: {}, }, - cardOptions: { - previewModifier: (s) => { - if (s.options?.reduceOptions?.values) { - s.options.reduceOptions.limit = 2; - } - }, + overrides: [], + }, + cardOptions: { + previewModifier: (s) => { + if (s.options?.reduceOptions?.values) { + s.options.reduceOptions.limit = 2; + } }, - }); + }, + } satisfies VisualizationSuggestion); - if (dataSummary.hasStringField && dataSummary.frameCount === 1 && dataSummary.rowCountTotal < 10) { - list.append({ - name: SuggestionName.Gauge, - options: { - reduceOptions: { - values: true, - calcs: [], - }, - }, - }); - list.append({ - name: SuggestionName.GaugeNoThresholds, - options: { - reduceOptions: { - values: true, - calcs: [], - }, - showThresholdMarkers: false, - }, - }); - } else { - list.append({ - name: SuggestionName.Gauge, - options: { - reduceOptions: { - values: false, - calcs: ['lastNotNull'], - }, - }, - }); - list.append({ - name: SuggestionName.GaugeNoThresholds, - options: { - reduceOptions: { - values: false, - calcs: ['lastNotNull'], - }, - showThresholdMarkers: false, - }, - }); - } +const GAUGE_LIMIT = 10; + +export const gaugeSuggestionsSupplier: VisualizationSuggestionsSupplierFn = (dataSummary) => { + if (!dataSummary.hasData || !dataSummary.hasFieldType(FieldType.number)) { + return; } -} + + // for many fields / series this is probably not a good fit + if (dataSummary.fieldCountByType(FieldType.number) > GAUGE_LIMIT) { + return; + } + + const suggestions: Array> = [ + { name: t('gauge.suggestions.arc', 'Gauge') }, + { + name: t('gauge.suggestions.no-thresholds', 'Gauge - no thresholds'), + options: { + showThresholdMarkers: false, + }, + }, + ]; + + // sometimes, we want to de-aggregate the data for the gauge suggestion + const shouldUseRawValues = + dataSummary.hasFieldType(FieldType.string) && + dataSummary.frameCount === 1 && + dataSummary.rowCountTotal <= GAUGE_LIMIT; + + return suggestions.map((s) => defaultNumericVizOptions(withDefaults(s), dataSummary, shouldUseRawValues)); +}; diff --git a/public/app/plugins/panel/geomap/module.tsx b/public/app/plugins/panel/geomap/module.tsx index 7573f9317aa..a88a3bfbe02 100644 --- a/public/app/plugins/panel/geomap/module.tsx +++ b/public/app/plugins/panel/geomap/module.tsx @@ -8,6 +8,7 @@ import { LayersEditor } from './editor/LayersEditor'; import { MapViewEditor } from './editor/MapViewEditor'; import { getLayerEditor } from './editor/layerEditor'; import { mapPanelChangedHandler, mapMigrationHandler } from './migrations'; +import { geomapSuggestionsSupplier } from './suggestions'; import { defaultMapViewConfig, Options, TooltipMode, GeomapInstanceState } from './types'; export const plugin = new PanelPlugin(GeomapPanel) @@ -171,4 +172,5 @@ export const plugin = new PanelPlugin(GeomapPanel) ], }, }); - }); + }) + .setSuggestionsSupplier(geomapSuggestionsSupplier); diff --git a/public/app/plugins/panel/geomap/suggestions.ts b/public/app/plugins/panel/geomap/suggestions.ts new file mode 100644 index 00000000000..97fe03a7739 --- /dev/null +++ b/public/app/plugins/panel/geomap/suggestions.ts @@ -0,0 +1,46 @@ +import { VisualizationSuggestionScore, VisualizationSuggestionsSupplierFn } from '@grafana/data'; +import { GraphFieldConfig } from '@grafana/ui'; +import { getGeometryField, getDefaultLocationMatchers } from 'app/features/geo/utils/location'; + +import { Options } from './panelcfg.gen'; + +export const geomapSuggestionsSupplier: VisualizationSuggestionsSupplierFn = ( + dataSummary +) => { + if (!dataSummary.hasData || !dataSummary.rawFrames) { + return; + } + + // use getGeometryField to see if any frames have geolocation info + const location = getDefaultLocationMatchers(); + if (!dataSummary.rawFrames.some((frame) => !getGeometryField(frame, location).warning)) { + return; + } + + return [ + { + score: VisualizationSuggestionScore.Best, + fieldConfig: { + defaults: { + custom: {}, + }, + overrides: [], + }, + cardOptions: { + previewModifier: (s) => { + s.options!.controls = { + showZoom: false, + showScale: false, + showAttribution: false, + showMeasure: false, + }; + // FIXME: this doesn't work. I want to disable legends in the preview. + s.options?.layers?.forEach((layer) => { + layer.config = layer.config || {}; + layer.config.showLegend = false; + }); + }, + }, + }, + ]; +}; diff --git a/public/app/plugins/panel/heatmap/module.tsx b/public/app/plugins/panel/heatmap/module.tsx index 897fdbda461..a7d44726886 100644 --- a/public/app/plugins/panel/heatmap/module.tsx +++ b/public/app/plugins/panel/heatmap/module.tsx @@ -18,7 +18,7 @@ import { HeatmapPanel } from './HeatmapPanel'; import { prepareHeatmapData } from './fields'; import { heatmapChangedHandler, heatmapMigrationHandler } from './migrations'; import { colorSchemes, quantizeScheme } from './palettes'; -import { HeatmapSuggestionsSupplier } from './suggestions'; +import { heatmapSuggestionsSupplier } from './suggestions'; import { Options, defaultOptions, HeatmapColorMode, HeatmapColorScale } from './types'; export const plugin = new PanelPlugin(HeatmapPanel) @@ -472,5 +472,5 @@ export const plugin = new PanelPlugin(HeatmapPanel) annotations?.some((df) => df.meta?.custom?.resultType === 'exemplar'), }); }) - .setSuggestionsSupplier(new HeatmapSuggestionsSupplier()) + .setSuggestionsSupplier(heatmapSuggestionsSupplier) .setDataSupport({ annotations: true }); diff --git a/public/app/plugins/panel/heatmap/suggestions.test.ts b/public/app/plugins/panel/heatmap/suggestions.test.ts new file mode 100644 index 00000000000..ef3e73bc0aa --- /dev/null +++ b/public/app/plugins/panel/heatmap/suggestions.test.ts @@ -0,0 +1,226 @@ +import { + createDataFrame, + DataFrameType, + FieldType, + getPanelDataSummary, + VisualizationSuggestionScore, +} from '@grafana/data'; + +import { heatmapSuggestionsSupplier } from './suggestions'; + +describe('heatmap suggestions', () => { + describe('applicability', () => { + it('should not suggest for data without time field', () => { + const dataSummary = getPanelDataSummary([ + createDataFrame({ + fields: [ + { name: 'value1', type: FieldType.number, values: [1, 2, 3] }, + { name: 'value2', type: FieldType.number, values: [4, 5, 6] }, + ], + }), + ]); + + const suggestions = heatmapSuggestionsSupplier(dataSummary); + expect(suggestions).toBeUndefined(); + }); + + it('should suggest for data with time and number fields', () => { + const dataSummary = getPanelDataSummary([ + createDataFrame({ + fields: [ + { name: 'time', type: FieldType.time, values: [1609459200000, 1609462800000, 1609466400000] }, + { name: 'value1', type: FieldType.number, values: [1, 2, 3] }, + { name: 'value2', type: FieldType.number, values: [4, 5, 6] }, + ], + }), + ]); + + const suggestions = heatmapSuggestionsSupplier(dataSummary); + expect(suggestions).toHaveLength(1); + }); + }); + + describe('scoring', () => { + it('should score this as "OK" if the data is not particularly heatmap-y', () => { + const dataSummary = getPanelDataSummary([ + createDataFrame({ + fields: [ + { name: 'time', type: FieldType.time, values: [1609459200000, 1609462800000, 1609466400000] }, + { name: 'value1', type: FieldType.number, values: [1, 2, 3] }, + { name: 'value2', type: FieldType.number, values: [4, 5, 6] }, + ], + }), + ]); + + const suggestions = heatmapSuggestionsSupplier(dataSummary); + expect(suggestions).toEqual([expect.objectContaining({ score: VisualizationSuggestionScore.OK })]); + }); + + it.each([DataFrameType.HeatmapRows, DataFrameType.HeatmapCells])( + 'should score this as "Best" if the data explicitly has %s frame type', + (frameType: DataFrameType) => { + const dataSummary = getPanelDataSummary([ + createDataFrame({ + meta: { type: frameType }, + fields: [ + { + name: 'time', + type: FieldType.time, + values: [1609459200000, 1609462800000, 1609466400000], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value1', + type: FieldType.number, + values: [1, 2, 3], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value2', + type: FieldType.number, + values: [4, 5, 6], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value3', + type: FieldType.number, + values: [7, 8, 9], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + ], + }), + ]); + + const suggestions = heatmapSuggestionsSupplier(dataSummary); + expect(suggestions).toEqual([expect.objectContaining({ score: VisualizationSuggestionScore.Best })]); + } + ); + + it('should score this as "Best" if the data has "ge" labels', () => { + const dataSummary = getPanelDataSummary([ + createDataFrame({ + fields: [ + { + name: 'time', + type: FieldType.time, + values: [1609459200000, 1609462800000, 1609466400000], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value1', + type: FieldType.number, + values: [1, 2, 3], + labels: { ge: '-Inf' }, + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value2', + type: FieldType.number, + values: [4, 5, 6], + labels: { ge: '0' }, + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value3', + type: FieldType.number, + values: [7, 8, 9], + labels: { ge: '10' }, + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + ], + }), + ]); + + const suggestions = heatmapSuggestionsSupplier(dataSummary); + expect(suggestions).toEqual([expect.objectContaining({ score: VisualizationSuggestionScore.Best })]); + }); + + it('should score this as "Best" if the data has "le" labels', () => { + const dataSummary = getPanelDataSummary([ + createDataFrame({ + fields: [ + { + name: 'time', + type: FieldType.time, + values: [1609459200000, 1609462800000, 1609466400000], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value1', + type: FieldType.number, + values: [1, 2, 3], + labels: { le: '1' }, + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value2', + type: FieldType.number, + values: [4, 5, 6], + labels: { le: '2' }, + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value2', + type: FieldType.number, + values: [6, 2, 6], + labels: { le: '4' }, + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'value3', + type: FieldType.number, + values: [7, 8, 9], + labels: { le: 'Inf' }, + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + ], + }), + ]); + + const suggestions = heatmapSuggestionsSupplier(dataSummary); + expect(suggestions).toEqual([expect.objectContaining({ score: VisualizationSuggestionScore.Best })]); + }); + + it('should score this as "Best" if the field names are numeric in a way that makes sense', () => { + const dataSummary = getPanelDataSummary([ + createDataFrame({ + fields: [ + { + name: 'time', + type: FieldType.time, + values: [1609459200000, 1609462800000, 1609466400000], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: '0', + type: FieldType.number, + values: [1, 2, 3], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: '10', + type: FieldType.number, + values: [4, 5, 6], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: '20', + type: FieldType.number, + values: [7, 8, 9], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + { + name: 'Inf', + type: FieldType.number, + values: [10, 9, 8], + display: jest.fn((v) => ({ text: '' + v, numeric: Number(v) })), + }, + ], + }), + ]); + + const suggestions = heatmapSuggestionsSupplier(dataSummary); + expect(suggestions).toEqual([expect.objectContaining({ score: VisualizationSuggestionScore.Best })]); + }); + }); +}); diff --git a/public/app/plugins/panel/heatmap/suggestions.ts b/public/app/plugins/panel/heatmap/suggestions.ts index f49de761e85..670f76cda63 100644 --- a/public/app/plugins/panel/heatmap/suggestions.ts +++ b/public/app/plugins/panel/heatmap/suggestions.ts @@ -1,45 +1,73 @@ -import { VisualizationSuggestionsBuilder } from '@grafana/data'; +import { + DataFrameType, + FieldType, + PanelDataSummary, + VisualizationSuggestionScore, + VisualizationSuggestionsSupplierFn, +} from '@grafana/data'; import { config } from '@grafana/runtime'; +import { GraphFieldConfig } from '@grafana/schema'; import { prepareHeatmapData } from './fields'; import { quantizeScheme } from './palettes'; import { Options, defaultOptions } from './types'; -export class HeatmapSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary } = builder; - - if ( - !builder.data?.series || - !dataSummary.hasData || - dataSummary.timeFieldCount < 1 || - dataSummary.numberFieldCount < 2 || - dataSummary.numberFieldCount > 10 - ) { - return; - } - - const palette = quantizeScheme(defaultOptions.color, config.theme2); - const info = prepareHeatmapData({ - frames: builder.data.series, - options: defaultOptions, - palette, - theme: config.theme2, - }); - if (!info || info.warning) { - return; - } - - builder.getListAppender({ - name: '', - pluginId: 'heatmap', - options: {}, - fieldConfig: { - defaults: { - custom: {}, - }, - overrides: [], - }, - }); +function determineScore(dataSummary: PanelDataSummary): VisualizationSuggestionScore { + // look to see if the data has an explicity marker for heatmap data on it. + if ([DataFrameType.HeatmapRows, DataFrameType.HeatmapCells].some((t) => dataSummary.hasDataFrameType(t))) { + return VisualizationSuggestionScore.Best; } + + // we'll also look more closely at frames which return between 3 and 10 numeric fields. + if (dataSummary.fieldCountByType(FieldType.number) > 2 || dataSummary.fieldCountByType(FieldType.number) <= 10) { + // look through the names of the panels + const hasPotentialHeatmapSeries = dataSummary.rawFrames!.some((frame) => { + for (const field of frame.fields) { + if (field.type === FieldType.number) { + // if the field name, or "ge" or "le" label on the field, are numeric, then it's very possibly part of a heatmap. + if ([field.name, field.labels?.ge, field.labels?.le].some((v) => !isNaN(Number(v)))) { + return true; + } + } + } + return false; + }); + + // if at least all but 1 of the numeric fields in the frame have numeric names, then this is probably a heatmap. + // (the out-of-place one would be "Inf" or "-Inf") + if (hasPotentialHeatmapSeries) { + return VisualizationSuggestionScore.Best; + } + } + + return VisualizationSuggestionScore.OK; } + +export const heatmapSuggestionsSupplier: VisualizationSuggestionsSupplierFn = ( + dataSummary: PanelDataSummary +) => { + if ( + !dataSummary.rawFrames || + !dataSummary.hasData || + !dataSummary.hasFieldType(FieldType.time) || + !dataSummary.hasFieldType(FieldType.number) + ) { + return; + } + + // parse the frame into a heatmap structure to see if it's possible. + const palette = quantizeScheme(defaultOptions.color, config.theme2); + const info = prepareHeatmapData({ + frames: dataSummary.rawFrames, + options: defaultOptions, + palette, + theme: config.theme2, + }); + + // if we can't parse the data into a heatmap, then bail out and prevent showing suggestions. + if (!info || info.warning) { + return; + } + + return [{ score: determineScore(dataSummary) }]; +}; diff --git a/public/app/plugins/panel/histogram/module.tsx b/public/app/plugins/panel/histogram/module.tsx index 4ee2fa97a09..8855287cc93 100644 --- a/public/app/plugins/panel/histogram/module.tsx +++ b/public/app/plugins/panel/histogram/module.tsx @@ -5,6 +5,9 @@ import { identityOverrideProcessor, PanelPlugin, histogramFieldInfo, + buildHistogram, + VisualizationSuggestionScore, + DataFrameType, } from '@grafana/data'; import { t } from '@grafana/i18n'; import { commonOptionsBuilder, getGraphFieldOptions } from '@grafana/ui'; @@ -149,4 +152,16 @@ export const plugin = new PanelPlugin(HistogramPanel) commonOptionsBuilder.addHideFrom(builder); }, + }) + .setSuggestionsSupplier((ds) => { + if (ds.rawFrames && ds.hasData && buildHistogram(ds.rawFrames)) { + return [ + { + score: ds.hasDataFrameType(DataFrameType.Histogram) + ? VisualizationSuggestionScore.Best + : VisualizationSuggestionScore.OK, + }, + ]; + } + return; }); diff --git a/public/app/plugins/panel/logs/module.tsx b/public/app/plugins/panel/logs/module.tsx index 04ed5e92183..e51ff946902 100644 --- a/public/app/plugins/panel/logs/module.tsx +++ b/public/app/plugins/panel/logs/module.tsx @@ -1,10 +1,10 @@ -import { PanelPlugin, LogsSortOrder, LogsDedupStrategy, LogsDedupDescription } from '@grafana/data'; +import { PanelPlugin, LogsSortOrder, LogsDedupStrategy, LogsDedupDescription, FieldType } from '@grafana/data'; import { t } from '@grafana/i18n'; import { config } from '@grafana/runtime'; +import { showDefaultSuggestion } from 'app/features/panel/suggestions/utils'; import { LogsPanel } from './LogsPanel'; import { Options } from './panelcfg.gen'; -import { LogsPanelSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(LogsPanel) .setPanelOptions((builder, context) => { @@ -207,4 +207,6 @@ export const plugin = new PanelPlugin(LogsPanel) defaultValue: LogsSortOrder.Descending, }); }) - .setSuggestionsSupplier(new LogsPanelSuggestionsSupplier()); + .setSuggestionsSupplier( + showDefaultSuggestion((ds) => ds.hasData && ds.hasFieldType(FieldType.time) && ds.hasFieldType(FieldType.string)) + ); diff --git a/public/app/plugins/panel/logs/suggestions.ts b/public/app/plugins/panel/logs/suggestions.ts deleted file mode 100644 index 79b804cb8d7..00000000000 --- a/public/app/plugins/panel/logs/suggestions.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { VisualizationSuggestionsBuilder, VisualizationSuggestionScore } from '@grafana/data'; -import { SuggestionName } from 'app/types/suggestions'; - -import { Options } from './panelcfg.gen'; - -export class LogsPanelSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const list = builder.getListAppender({ - name: '', - pluginId: 'logs', - options: {}, - fieldConfig: { - defaults: { - custom: {}, - }, - overrides: [], - }, - }); - - const { dataSummary: ds } = builder; - - // Require a string & time field - if (!ds.hasData || !ds.hasTimeField || !ds.hasStringField) { - return; - } - - if (ds.preferredVisualisationType === 'logs') { - list.append({ name: SuggestionName.Logs, score: VisualizationSuggestionScore.Best }); - } else { - list.append({ name: SuggestionName.Logs }); - } - } -} diff --git a/public/app/plugins/panel/nodeGraph/module.tsx b/public/app/plugins/panel/nodeGraph/module.tsx index 9bfa3456fc4..4230b30a9a1 100644 --- a/public/app/plugins/panel/nodeGraph/module.tsx +++ b/public/app/plugins/panel/nodeGraph/module.tsx @@ -4,7 +4,7 @@ import { t } from '@grafana/i18n'; import { NodeGraphPanel } from './NodeGraphPanel'; import { ArcOptionsEditor } from './editor/ArcOptionsEditor'; import { LayoutAlgorithm, Options as NodeGraphOptions } from './panelcfg.gen'; -import { NodeGraphSuggestionsSupplier } from './suggestions'; +import { nodeGraphSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(NodeGraphPanel) .useFieldConfig({ @@ -92,4 +92,4 @@ export const plugin = new PanelPlugin(NodeGraphPanel) }, }); }) - .setSuggestionsSupplier(new NodeGraphSuggestionsSupplier()); + .setSuggestionsSupplier(nodeGraphSuggestionsSupplier); diff --git a/public/app/plugins/panel/nodeGraph/suggestions.ts b/public/app/plugins/panel/nodeGraph/suggestions.ts index eb4a4773ef4..d1e5e1e29b4 100644 --- a/public/app/plugins/panel/nodeGraph/suggestions.ts +++ b/public/app/plugins/panel/nodeGraph/suggestions.ts @@ -1,71 +1,60 @@ -import { DataFrame, FieldType, VisualizationSuggestionsBuilder, VisualizationSuggestionScore } from '@grafana/data'; -import { SuggestionName } from 'app/types/suggestions'; +import { DataFrame, FieldType, VisualizationSuggestionScore, VisualizationSuggestionsSupplierFn } from '@grafana/data'; -export class NodeGraphSuggestionsSupplier { - getListWithDefaults(builder: VisualizationSuggestionsBuilder) { - return builder.getListAppender<{}, {}>({ - name: SuggestionName.NodeGraph, - pluginId: 'nodeGraph', - }); - } +import { Options } from './panelcfg.gen'; - hasCorrectFields(frames: DataFrame[]): boolean { - let hasNodesFrame = false; - let hasEdgesFrame = false; +function checkFields(fields: Array<[string, FieldType]>, frame: DataFrame): boolean { + let hasCorrectFields = true; - const nodeFields: Array<[string, FieldType]> = [ - ['id', FieldType.string], - ['title', FieldType.string], - ['mainstat', FieldType.number], - ]; - const edgeFields: Array<[string, FieldType]> = [ - ['id', FieldType.string], - ['source', FieldType.string], - ['target', FieldType.string], - ]; - - for (const frame of frames) { - if (this.checkFields(nodeFields, frame)) { - hasNodesFrame = true; - } - if (this.checkFields(edgeFields, frame)) { - hasEdgesFrame = true; - } - } - - return hasNodesFrame && hasEdgesFrame; - } - - checkFields(fields: Array<[string, FieldType]>, frame: DataFrame): boolean { - let hasCorrectFields = true; - - for (const field of fields) { - const [name, type] = field; - const frameField = frame.fields.find((f) => f.name === name); - if (!frameField || type !== frameField.type) { - hasCorrectFields = false; - break; - } - } - - return hasCorrectFields; - } - - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - if (!builder.data) { - return; - } - - const hasCorrectFields = this.hasCorrectFields(builder.data.series); - const nodeGraphFrames = builder.data.series.filter( - (df) => df.meta && df.meta.preferredVisualisationType === 'nodeGraph' - ); - - if (hasCorrectFields || nodeGraphFrames.length === 2) { - this.getListWithDefaults(builder).append({ - name: SuggestionName.NodeGraph, - score: VisualizationSuggestionScore.Best, - }); + for (const field of fields) { + const [name, type] = field; + const frameField = frame.fields.find((f) => f.name === name); + if (!frameField || type !== frameField.type) { + hasCorrectFields = false; + break; } } + + return hasCorrectFields; } + +function frameHasCorrectFields(frames: DataFrame[]): boolean { + let hasNodesFrame = false; + let hasEdgesFrame = false; + + const nodeFields: Array<[string, FieldType]> = [ + ['id', FieldType.string], + ['title', FieldType.string], + ['mainstat', FieldType.number], + ]; + const edgeFields: Array<[string, FieldType]> = [ + ['id', FieldType.string], + ['source', FieldType.string], + ['target', FieldType.string], + ]; + + for (const frame of frames) { + if (checkFields(nodeFields, frame)) { + hasNodesFrame = true; + } + if (checkFields(edgeFields, frame)) { + hasEdgesFrame = true; + } + } + + return hasNodesFrame && hasEdgesFrame; +} + +export const nodeGraphSuggestionsSupplier: VisualizationSuggestionsSupplierFn = (dataSummary) => { + if (!dataSummary.rawFrames) { + return; + } + + const hasCorrectFields = frameHasCorrectFields(dataSummary.rawFrames); + const nodeGraphFrames = dataSummary.hasPreferredVisualisationType('nodeGraph'); + + if (!hasCorrectFields && !nodeGraphFrames) { + return; + } + + return [{ score: VisualizationSuggestionScore.Best }]; +}; diff --git a/public/app/plugins/panel/piechart/module.tsx b/public/app/plugins/panel/piechart/module.tsx index 33726bf3a0f..683a0cc0b6a 100644 --- a/public/app/plugins/panel/piechart/module.tsx +++ b/public/app/plugins/panel/piechart/module.tsx @@ -9,7 +9,7 @@ import { addStandardDataReduceOptions } from '../stat/common'; import { PieChartPanel } from './PieChartPanel'; import { PieChartPanelChangedHandler } from './migrations'; import { Options, FieldConfig, PieChartType, PieChartLabels, PieChartLegendValues } from './panelcfg.gen'; -import { PieChartSuggestionsSupplier } from './suggestions'; +import { piechartSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(PieChartPanel) .setPanelChangeHandler(PieChartPanelChangedHandler) @@ -92,4 +92,4 @@ export const plugin = new PanelPlugin(PieChartPanel) showIf: (c) => c.legend.showLegend !== false, }); }) - .setSuggestionsSupplier(new PieChartSuggestionsSupplier()); + .setSuggestionsSupplier(piechartSuggestionsSupplier); diff --git a/public/app/plugins/panel/piechart/suggestions.ts b/public/app/plugins/panel/piechart/suggestions.ts index fd4beed5e6a..405d5722796 100644 --- a/public/app/plugins/panel/piechart/suggestions.ts +++ b/public/app/plugins/panel/piechart/suggestions.ts @@ -1,79 +1,78 @@ -import { VisualizationSuggestionsBuilder } from '@grafana/data'; +import { defaultsDeep } from 'lodash'; + +import { + FieldType, + VisualizationSuggestion, + VisualizationSuggestionScore, + VisualizationSuggestionsSupplierFn, +} from '@grafana/data'; +import { t } from '@grafana/i18n'; import { LegendDisplayMode } from '@grafana/schema'; -import { SuggestionName } from 'app/types/suggestions'; +import { defaultNumericVizOptions } from 'app/features/panel/suggestions/utils'; import { PieChartLabels, Options, PieChartType } from './panelcfg.gen'; -export class PieChartSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const list = builder.getListAppender({ - name: SuggestionName.PieChart, - pluginId: 'piechart', - options: { - reduceOptions: { - values: false, - calcs: ['lastNotNull'], - }, - displayLabels: [PieChartLabels.Percent], - legend: { - calcs: [], - displayMode: LegendDisplayMode.Hidden, - placement: 'right', - values: [], - showLegend: false, - }, +const withDefaults = (suggestion: VisualizationSuggestion): VisualizationSuggestion => + defaultsDeep(suggestion, { + options: { + displayLabels: [PieChartLabels.Percent], + legend: { + calcs: [], + displayMode: LegendDisplayMode.Hidden, + placement: 'right', + values: [], + showLegend: false, }, - }); + }, + } satisfies VisualizationSuggestion); - const { dataSummary } = builder; +const SLICE_MAX = 30; +const SLICE_MIN = 2; - if (!dataSummary.hasNumberField) { - return; - } +export const piechartSuggestionsSupplier: VisualizationSuggestionsSupplierFn = (dataSummary) => { + if (!dataSummary.hasFieldType(FieldType.number)) { + return; + } - if (dataSummary.hasStringField && dataSummary.frameCount === 1) { - // if many values this or single value PieChart is not a good option - if (dataSummary.rowCountTotal > 30 || dataSummary.rowCountTotal < 2) { - return; - } - - list.append({ - name: SuggestionName.PieChart, - options: { - reduceOptions: { - values: true, - calcs: [], - }, - }, - }); - - list.append({ - name: SuggestionName.PieChartDonut, - options: { - reduceOptions: { - values: true, - calcs: [], - }, - pieType: PieChartType.Donut, - }, - }); - - return; - } - - if (dataSummary.numberFieldCount > 30 || dataSummary.numberFieldCount < 2) { - return; - } - - list.append({ - name: SuggestionName.PieChart, - }); - - list.append({ - name: SuggestionName.PieChartDonut, + const suggestions: Array> = [ + { + name: t('piechart.suggestions.pie', 'Pie chart'), + }, + { + name: t('piechart.suggestions.donut', 'Donut chart'), options: { pieType: PieChartType.Donut, }, - }); + }, + ]; + + let shouldUseRawValues = false; + + // we're filtering out data which has more than 30 slices or less than 2, and we're also + // determining whether the reduce options should be set based on the data summary. + if (dataSummary.hasFieldType(FieldType.string) && dataSummary.frameCount === 1) { + if (dataSummary.rowCountTotal > SLICE_MAX && dataSummary.rowCountTotal < SLICE_MIN) { + return; + } + + shouldUseRawValues = true; + } else if ( + dataSummary.fieldCountByType(FieldType.number) > SLICE_MAX || + dataSummary.fieldCountByType(FieldType.number) < SLICE_MIN + ) { + return; } -} + + return suggestions.map((s) => { + const result = defaultNumericVizOptions(withDefaults(s), dataSummary, shouldUseRawValues); + // bump the score up to best if we have exactly one numeric and one string field + if ( + dataSummary.fieldCount === 2 && + dataSummary.fieldCountByType(FieldType.string) === 1 && + dataSummary.fieldCountByType(FieldType.number) === 1 + ) { + result.score = VisualizationSuggestionScore.Best; + } + return result; + }); +}; diff --git a/public/app/plugins/panel/radialbar/module.tsx b/public/app/plugins/panel/radialbar/module.tsx index dfbdb875865..33212d3ce1c 100644 --- a/public/app/plugins/panel/radialbar/module.tsx +++ b/public/app/plugins/panel/radialbar/module.tsx @@ -8,7 +8,7 @@ import { EffectsEditor } from './EffectsEditor'; import { gaugePanelChangedHandler, gaugePanelMigrationHandler, shouldMigrateGauge } from './GaugeMigrations'; import { RadialBarPanel } from './RadialBarPanel'; import { defaultGaugePanelEffects, defaultOptions, Options } from './panelcfg.gen'; -import { radialBarSuggestionsHandler } from './suggestions'; +import { radialBarSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(RadialBarPanel) .useFieldConfig({}) @@ -112,6 +112,6 @@ export const plugin = new PanelPlugin(RadialBarPanel) defaultValue: defaultGaugePanelEffects, }); }) - .setSuggestionsSupplier(radialBarSuggestionsHandler) + .setSuggestionsSupplier(radialBarSuggestionsSupplier) .setMigrationHandler(gaugePanelMigrationHandler, shouldMigrateGauge) .setPanelChangeHandler(gaugePanelChangedHandler); diff --git a/public/app/plugins/panel/radialbar/suggestions.test.ts b/public/app/plugins/panel/radialbar/suggestions.test.ts index 21a59267bbb..0d8ead08c87 100644 --- a/public/app/plugins/panel/radialbar/suggestions.test.ts +++ b/public/app/plugins/panel/radialbar/suggestions.test.ts @@ -1,13 +1,13 @@ import { createDataFrame, Field, FieldType, getPanelDataSummary } from '@grafana/data'; -import { radialBarSuggestionsHandler } from './suggestions'; +import { radialBarSuggestionsSupplier } from './suggestions'; describe('RadialBarPanel Suggestions', () => { it('does not suggest gauge if no data is present', () => { - expect(radialBarSuggestionsHandler(getPanelDataSummary([]))).toBeFalsy(); - expect(radialBarSuggestionsHandler(getPanelDataSummary(undefined))).toBeFalsy(); + expect(radialBarSuggestionsSupplier(getPanelDataSummary([]))).toBeFalsy(); + expect(radialBarSuggestionsSupplier(getPanelDataSummary(undefined))).toBeFalsy(); expect( - radialBarSuggestionsHandler( + radialBarSuggestionsSupplier( getPanelDataSummary([ createDataFrame({ fields: [ @@ -27,7 +27,7 @@ describe('RadialBarPanel Suggestions', () => { { name: 'status', type: FieldType.string }, ], }); - expect(radialBarSuggestionsHandler(getPanelDataSummary([df]))).toBeFalsy(); + expect(radialBarSuggestionsSupplier(getPanelDataSummary([df]))).toBeFalsy(); }); it('does not suggest gauge if there are too many numeric fields', () => { @@ -35,12 +35,12 @@ describe('RadialBarPanel Suggestions', () => { for (let i = 0; i < 20; i++) { fields.push({ name: `numeric-${i}`, type: FieldType.number, values: [0, 100, 200, 300, 400, 500], config: {} }); } - expect(radialBarSuggestionsHandler(getPanelDataSummary([createDataFrame({ fields })]))).toBeFalsy(); + expect(radialBarSuggestionsSupplier(getPanelDataSummary([createDataFrame({ fields })]))).toBeFalsy(); }); it('suggests gauge for a single numeric field', () => { expect( - radialBarSuggestionsHandler( + radialBarSuggestionsSupplier( getPanelDataSummary([ createDataFrame({ fields: [ @@ -58,7 +58,7 @@ describe('RadialBarPanel Suggestions', () => { it('suggests gauge for a few numeric fields, with other fields mixed in', () => { expect( - radialBarSuggestionsHandler( + radialBarSuggestionsSupplier( getPanelDataSummary([ createDataFrame({ fields: [ @@ -136,7 +136,7 @@ describe('RadialBarPanel Suggestions', () => { ], }, ])('$description suggests aggregated=$aggregated', ({ dataframes, aggregated }) => { - const suggestions = radialBarSuggestionsHandler(getPanelDataSummary(dataframes)); + const suggestions = radialBarSuggestionsSupplier(getPanelDataSummary(dataframes)); const expected = aggregated ? { values: false, calcs: ['lastNotNull'] } : { values: true, calcs: [] }; if (Array.isArray(suggestions)) { for (const suggestion of suggestions) { diff --git a/public/app/plugins/panel/radialbar/suggestions.ts b/public/app/plugins/panel/radialbar/suggestions.ts index 35cde48ec0f..f8d13a096b3 100644 --- a/public/app/plugins/panel/radialbar/suggestions.ts +++ b/public/app/plugins/panel/radialbar/suggestions.ts @@ -8,10 +8,39 @@ import { } from '@grafana/data'; import { t } from '@grafana/i18n'; import { GraphFieldConfig } from '@grafana/ui'; +import { defaultNumericVizOptions } from 'app/features/panel/suggestions/utils'; import { Options } from './panelcfg.gen'; -export const radialBarSuggestionsHandler: VisualizationSuggestionsSupplierFn = ( +const withDefaults = ( + suggestion: VisualizationSuggestion +): VisualizationSuggestion => + defaultsDeep(suggestion, { + cardOptions: { + previewModifier: (s) => { + if (s.options?.reduceOptions) { + s.options.reduceOptions.limit = 4; + } + }, + }, + // styles: [{ + // name: t('gauge.suggestions.style.circular', 'Glowing'), + // options: { + // effects: { + // rounded: true, + // barGlow: true, + // centerGlow: true, + // spotlight: true, + // }, + // }, + // }, { + // name: t('gauge.suggestions.style.simple', 'Simple'), + // }] + } satisfies VisualizationSuggestion); + +const MAX_GAUGES = 10; + +export const radialBarSuggestionsSupplier: VisualizationSuggestionsSupplierFn = ( dataSummary ) => { if (!dataSummary.hasData || !dataSummary.hasFieldType(FieldType.number)) { @@ -19,69 +48,40 @@ export const radialBarSuggestionsHandler: VisualizationSuggestionsSupplierFn= 10) { + if (dataSummary.fieldCountByType(FieldType.number) > MAX_GAUGES) { return; } - const withDefaults = ( - suggestion: VisualizationSuggestion - ): VisualizationSuggestion => { - // if there is a string field and there are few enough rows, we assume it's tabular data and not numeric series data, - // and the de-aggregated version of the viz probably makes more sense - const isTabularData = - dataSummary.hasFieldType(FieldType.string) && dataSummary.frameCount === 1 && dataSummary.rowCountTotal < 10; - return defaultsDeep(suggestion, { - options: { - reduceOptions: isTabularData - ? { - values: true, - calcs: [], - } - : { - values: false, - calcs: ['lastNotNull'], - }, - }, - fieldConfig: { - defaults: isTabularData - ? { - color: { mode: FieldColorModeId.PaletteClassic }, - } - : {}, - overrides: [], - }, - cardOptions: { - previewModifier: (s) => { - if (s.options?.reduceOptions) { - s.options.reduceOptions.limit = 4; - } - }, - }, - // styles: [{ - // name: t('gauge.suggestions.style.circular', 'Glowing'), - // options: { - // effects: { - // rounded: true, - // barGlow: true, - // centerGlow: true, - // spotlight: true, - // }, - // }, - // }, { - // name: t('gauge.suggestions.style.simple', 'Simple'), - // }] - } satisfies VisualizationSuggestion); - }; - - return [ - withDefaults({ name: t('gauge.suggestions.arc', 'Gauge') }), - withDefaults({ + const suggestions: Array> = [ + { name: t('gauge.suggestions.arc', 'Gauge') }, + { name: t('gauge.suggestions.circular', 'Circular gauge'), options: { shape: 'circle', showThresholdMarkers: false, barWidthFactor: 0.3, }, - }), + }, ]; + + const shouldUseRawValues = + dataSummary.hasFieldType(FieldType.string) && + dataSummary.frameCount === 1 && + dataSummary.rowCountTotal <= MAX_GAUGES; + + return suggestions.map((s) => { + const suggestion = defaultNumericVizOptions(withDefaults(s), dataSummary, shouldUseRawValues); + + if (shouldUseRawValues) { + suggestion.fieldConfig = suggestion.fieldConfig ?? { + defaults: {}, + overrides: [], + }; + suggestion.fieldConfig.defaults.color = suggestion.fieldConfig.defaults.color ?? { + mode: FieldColorModeId.PaletteClassic, + }; + } + + return suggestion; + }); }; diff --git a/public/app/plugins/panel/stat/module.tsx b/public/app/plugins/panel/stat/module.tsx index 969e010352c..ecb146a851b 100644 --- a/public/app/plugins/panel/stat/module.tsx +++ b/public/app/plugins/panel/stat/module.tsx @@ -13,7 +13,7 @@ import { statPanelChangedHandler } from './StatMigrations'; import { StatPanel } from './StatPanel'; import { addStandardDataReduceOptions, addOrientationOption } from './common'; import { defaultOptions, Options } from './panelcfg.gen'; -import { StatSuggestionsSupplier } from './suggestions'; +import { statSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(StatPanel) .useFieldConfig() @@ -137,5 +137,5 @@ export const plugin = new PanelPlugin(StatPanel) }) .setNoPadding() .setPanelChangeHandler(statPanelChangedHandler) - .setSuggestionsSupplier(new StatSuggestionsSupplier()) + .setSuggestionsSupplier(statSuggestionsSupplier) .setMigrationHandler(sharedSingleStatMigrationHandler); diff --git a/public/app/plugins/panel/stat/suggestions.ts b/public/app/plugins/panel/stat/suggestions.ts index 00af3220135..14a45d4b52d 100644 --- a/public/app/plugins/panel/stat/suggestions.ts +++ b/public/app/plugins/panel/stat/suggestions.ts @@ -1,41 +1,46 @@ -import { VisualizationSuggestionsBuilder } from '@grafana/data'; -import { BigValueColorMode, BigValueGraphMode, GraphFieldConfig } from '@grafana/schema'; -import { SuggestionName } from 'app/types/suggestions'; +import { defaultsDeep } from 'lodash'; + +import { FieldType, VisualizationSuggestion, VisualizationSuggestionsSupplierFn } from '@grafana/data'; +import { t } from '@grafana/i18n'; +import { BigValueColorMode, BigValueGraphMode } from '@grafana/schema'; import { Options } from './panelcfg.gen'; -export class StatSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary: ds } = builder; - - if (!ds.hasData) { - return; - } - - const list = builder.getListAppender({ - name: SuggestionName.Stat, - pluginId: 'stat', - options: {}, - fieldConfig: { - defaults: { - unit: 'short', - custom: {}, - }, - overrides: [], +const withDefaults = (s: VisualizationSuggestion): VisualizationSuggestion => + defaultsDeep(s, { + fieldConfig: { + defaults: { + unit: 'short', + custom: {}, }, - cardOptions: { - previewModifier: (s) => { - if (s.options?.reduceOptions?.values) { - s.options.reduceOptions.limit = 1; - } - }, + overrides: [], + }, + cardOptions: { + previewModifier: (s) => { + if (s.options?.reduceOptions?.values) { + s.options.reduceOptions.limit = 1; + } }, - }); + }, + } satisfies VisualizationSuggestion); - // String and number field with low row count show individual rows - if (ds.hasStringField && ds.hasNumberField && ds.frameCount === 1 && ds.rowCountTotal < 10) { - list.append({ - name: SuggestionName.Stat, +export const statSuggestionsSupplier: VisualizationSuggestionsSupplierFn = (ds) => { + if (!ds.hasData) { + return; + } + + const suggestions: Array> = []; + + // String and number field with low row count show individual rows + if ( + ds.hasFieldType(FieldType.string) && + ds.hasFieldType(FieldType.number) && + ds.frameCount === 1 && + ds.rowCountTotal < 10 + ) { + suggestions.push( + { + name: t('stat.suggestions.stat-discrete-values', 'Stat - discrete values'), options: { reduceOptions: { values: true, @@ -43,9 +48,9 @@ export class StatSuggestionsSupplier { fields: '/.*/', }, }, - }); - list.append({ - name: SuggestionName.StatColoredBackground, + }, + { + name: t('stat.suggestions.stat-discrete-values-color-background', 'Stat - discrete values - color background'), options: { reduceOptions: { values: true, @@ -54,36 +59,38 @@ export class StatSuggestionsSupplier { }, colorMode: BigValueColorMode.Background, }, - }); - } + } + ); + } - // Just a single string field - if (ds.stringFieldCount === 1 && ds.frameCount === 1 && ds.rowCountTotal < 10 && ds.fieldCount === 1) { - list.append({ - name: SuggestionName.Stat, - options: { - reduceOptions: { - values: true, - calcs: [], - fields: '/.*/', - }, - colorMode: BigValueColorMode.None, + // just a single string field + if (ds.fieldCount === 1 && ds.hasFieldType(FieldType.string)) { + suggestions.push({ + name: t('stat.suggestions.stat-single-string', 'Stat - single string'), + options: { + reduceOptions: { + values: true, + calcs: [], + fields: '/.*/', }, - }); - } + colorMode: BigValueColorMode.None, + }, + }); + } - if (ds.hasNumberField && ds.hasTimeField) { - list.append({ + // aggregated suggestions for number fields + if (ds.hasFieldType(FieldType.number) && ds.hasFieldType(FieldType.time)) { + suggestions.push( + { options: { reduceOptions: { values: false, calcs: ['lastNotNull'], }, }, - }); - - list.append({ - name: SuggestionName.StatColoredBackground, + }, + { + name: t('stat.suggestions.stat-color-background', 'Stat - color background'), options: { reduceOptions: { values: false, @@ -92,7 +99,9 @@ export class StatSuggestionsSupplier { graphMode: BigValueGraphMode.None, colorMode: BigValueColorMode.Background, }, - }); - } + } + ); } -} + + return suggestions.map(withDefaults); +}; diff --git a/public/app/plugins/panel/state-timeline/module.tsx b/public/app/plugins/panel/state-timeline/module.tsx index 55fd557eea0..89acb338825 100644 --- a/public/app/plugins/panel/state-timeline/module.tsx +++ b/public/app/plugins/panel/state-timeline/module.tsx @@ -177,7 +177,7 @@ export const plugin = new PanelPlugin(StateTimelinePanel) } // Probably better ways to filter out this by inspecting the types of string values so view this as temporary - if (ds.preferredVisualisationType === 'logs') { + if (ds.hasPreferredVisualisationType('logs')) { return; } diff --git a/public/app/plugins/panel/status-history/module.tsx b/public/app/plugins/panel/status-history/module.tsx index 1e07588d664..eed5e3546a8 100644 --- a/public/app/plugins/panel/status-history/module.tsx +++ b/public/app/plugins/panel/status-history/module.tsx @@ -1,11 +1,10 @@ -import { FieldColorModeId, FieldConfigProperty, PanelPlugin } from '@grafana/data'; +import { FieldColorModeId, FieldConfigProperty, FieldType, PanelPlugin } from '@grafana/data'; import { t } from '@grafana/i18n'; import { AxisPlacement, VisibilityMode } from '@grafana/schema'; import { commonOptionsBuilder } from '@grafana/ui'; import { StatusHistoryPanel } from './StatusHistoryPanel'; import { Options, FieldConfig, defaultFieldConfig } from './panelcfg.gen'; -import { StatusHistorySuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(StatusHistoryPanel) .useFieldConfig({ @@ -113,5 +112,42 @@ export const plugin = new PanelPlugin(StatusHistoryPanel) commonOptionsBuilder.addLegendOptions(builder, false); commonOptionsBuilder.addTooltipOptions(builder); }) - .setSuggestionsSupplier(new StatusHistorySuggestionsSupplier()) + .setSuggestionsSupplier((ds) => { + if (!ds.hasData) { + return; + } + + // This panel needs a time field and a string or number field + if ( + !ds.hasFieldType(FieldType.time) || + (!ds.hasFieldType(FieldType.string) && !ds.hasFieldType(FieldType.number)) + ) { + return; + } + + // If there are many series then they won't fit on y-axis so this panel is not good fit + if (ds.fieldCountByType(FieldType.number) >= 30) { + return; + } + + // if there a lot of data points for each series then this is not a good match + if (ds.rowCountMax > 100) { + return; + } + + // Probably better ways to filter out this by inspecting the types of string values so view this as temporary + if (ds.hasPreferredVisualisationType('logs')) { + return; + } + + return [ + { + cardOptions: { + previewModifier: (s) => { + s.options!.colWidth = 0.7; + }, + }, + }, + ]; + }) .setDataSupport({ annotations: true }); diff --git a/public/app/plugins/panel/status-history/suggestions.ts b/public/app/plugins/panel/status-history/suggestions.ts deleted file mode 100644 index 48add53c83d..00000000000 --- a/public/app/plugins/panel/status-history/suggestions.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { FieldColorModeId, VisualizationSuggestionsBuilder } from '@grafana/data'; -import { SuggestionName } from 'app/types/suggestions'; - -import { Options, FieldConfig } from './panelcfg.gen'; - -export class StatusHistorySuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary: ds } = builder; - - if (!ds.hasData) { - return; - } - - // This panel needs a time field and a string or number field - if (!ds.hasTimeField || (!ds.hasStringField && !ds.hasNumberField)) { - return; - } - - // If there are many series then they won't fit on y-axis so this panel is not good fit - if (ds.numberFieldCount >= 30) { - return; - } - - // if there a lot of data points for each series then this is not a good match - if (ds.rowCountMax > 100) { - return; - } - - // Probably better ways to filter out this by inspecting the types of string values so view this as temporary - if (ds.preferredVisualisationType === 'logs') { - return; - } - - const list = builder.getListAppender({ - name: '', - pluginId: 'status-history', - options: {}, - fieldConfig: { - defaults: { - color: { - mode: FieldColorModeId.ContinuousGrYlRd, - }, - custom: {}, - }, - overrides: [], - }, - cardOptions: { - previewModifier: (s) => { - s.options!.colWidth = 0.7; - }, - }, - }); - - list.append({ name: SuggestionName.StatusHistory }); - } -} diff --git a/public/app/plugins/panel/table/module.tsx b/public/app/plugins/panel/table/module.tsx index e2ee5b8e97f..5a3a1d21fac 100644 --- a/public/app/plugins/panel/table/module.tsx +++ b/public/app/plugins/panel/table/module.tsx @@ -13,7 +13,7 @@ import { TableCellOptionEditor } from './TableCellOptionEditor'; import { TablePanel } from './TablePanel'; import { tableMigrationHandler, tablePanelChangedHandler } from './migrations'; import { Options, defaultOptions, FieldConfig } from './panelcfg.gen'; -import { TableSuggestionsSupplier } from './suggestions'; +import { tableSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(TablePanel) .setPanelChangeHandler(tablePanelChangedHandler) @@ -227,4 +227,4 @@ export const plugin = new PanelPlugin(TablePanel) defaultValue: defaultOptions?.enablePagination, }); }) - .setSuggestionsSupplier(new TableSuggestionsSupplier()); + .setSuggestionsSupplier(tableSuggestionsSupplier); diff --git a/public/app/plugins/panel/table/suggestions.ts b/public/app/plugins/panel/table/suggestions.ts index bcfead0fe31..98fc085e1cf 100644 --- a/public/app/plugins/panel/table/suggestions.ts +++ b/public/app/plugins/panel/table/suggestions.ts @@ -1,38 +1,33 @@ -import { VisualizationSuggestionsBuilder } from '@grafana/data'; -import { TableFieldOptions } from '@grafana/schema'; +import { PanelDataSummary, VisualizationSuggestionScore, VisualizationSuggestionsSupplierFn } from '@grafana/data'; import icnTablePanelSvg from 'app/plugins/panel/table/img/icn-table-panel.svg'; -import { SuggestionName } from 'app/types/suggestions'; -import { Options } from './panelcfg.gen'; +import { Options, FieldConfig } from './panelcfg.gen'; -export class TableSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const list = builder.getListAppender({ - name: SuggestionName.Table, - pluginId: 'table', - options: {}, - fieldConfig: { - defaults: { - custom: {}, - }, - overrides: [], - }, - cardOptions: { - previewModifier: (s) => { - s.fieldConfig!.defaults.custom!.minWidth = 50; - }, - }, - }); - - // If there are not data suggest table anyway but use icon instead of real preview - if (builder.dataSummary.fieldCount === 0) { - list.append({ - cardOptions: { - imgSrc: icnTablePanelSvg, - }, - }); - } else { - list.append({}); - } +function getTableSuggestionScore(dataSummary: PanelDataSummary): VisualizationSuggestionScore { + if (dataSummary.hasPreferredVisualisationType('table')) { + return VisualizationSuggestionScore.Best; } + + // table is best suited to showing many fields with many rows. + if (dataSummary.fieldCountMax > 5 && dataSummary.rowCountMax > 50) { + return VisualizationSuggestionScore.Good; + } + + return VisualizationSuggestionScore.OK; } + +export const tableSuggestionsSupplier: VisualizationSuggestionsSupplierFn = (dataSummary) => [ + { + score: getTableSuggestionScore(dataSummary), + cardOptions: { + previewModifier: (s) => { + if (s.fieldConfig && s.fieldConfig.defaults.custom) { + s.fieldConfig.defaults.custom.minWidth = 50; + } + }, + // If there is no data, suggest table anyway, but use icon instead of real preview + // TODO: delete this in once "new" suggestions are fully rolled out + imgSrc: dataSummary.fieldCount === 0 ? icnTablePanelSvg : undefined, + }, + }, +]; diff --git a/public/app/plugins/panel/timeseries/module.tsx b/public/app/plugins/panel/timeseries/module.tsx index beccdf480d6..22b0e3a715a 100644 --- a/public/app/plugins/panel/timeseries/module.tsx +++ b/public/app/plugins/panel/timeseries/module.tsx @@ -8,7 +8,7 @@ import { TimezonesEditor } from './TimezonesEditor'; import { defaultGraphConfig, getGraphFieldConfig } from './config'; import { graphPanelChangedHandler } from './migrations'; import { FieldConfig, Options } from './panelcfg.gen'; -import { TimeSeriesSuggestionsSupplier } from './suggestions'; +import { timeseriesSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(TimeSeriesPanel) .setPanelChangeHandler(graphPanelChangedHandler) @@ -26,5 +26,5 @@ export const plugin = new PanelPlugin(TimeSeriesPanel) defaultValue: undefined, }); }) - .setSuggestionsSupplier(new TimeSeriesSuggestionsSupplier()) + .setSuggestionsSupplier(timeseriesSuggestionsSupplier) .setDataSupport({ annotations: true, alertStates: true }); diff --git a/public/app/plugins/panel/timeseries/suggestions.ts b/public/app/plugins/panel/timeseries/suggestions.ts index d41179d3f36..05a73acdcc7 100644 --- a/public/app/plugins/panel/timeseries/suggestions.ts +++ b/public/app/plugins/panel/timeseries/suggestions.ts @@ -1,9 +1,15 @@ +import { defaultsDeep } from 'lodash'; + import { - FieldColorModeId, - VisualizationSuggestionsBuilder, + DataFrameType, DataTransformerID, + FieldType, PanelPluginVisualizationSuggestion, + VisualizationSuggestion, + VisualizationSuggestionScore, + VisualizationSuggestionsSupplierFn, } from '@grafana/data'; +import { t } from '@grafana/i18n'; import { GraphDrawStyle, GraphFieldConfig, @@ -13,209 +19,142 @@ import { StackingMode, } from '@grafana/schema'; import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; -import { SuggestionName } from 'app/types/suggestions'; import { Options } from './panelcfg.gen'; -export class TimeSeriesSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary } = builder; +const MAX_BARS = 100; +const MAX_ROWS_SMOOTH_CHART = 200; - if (!dataSummary.hasTimeField || !dataSummary.hasNumberField || dataSummary.rowCountTotal < 2) { - return; - } - - const list = builder.getListAppender({ - name: SuggestionName.LineChart, - pluginId: 'timeseries', - options: { - legend: { - calcs: [], - displayMode: LegendDisplayMode.Hidden, - placement: 'right', - showLegend: false, - }, +const withDefaults = ( + suggestion: VisualizationSuggestion +): VisualizationSuggestion => + defaultsDeep(suggestion, { + options: { + legend: { + calcs: [], + displayMode: LegendDisplayMode.Hidden, + placement: 'right', + showLegend: false, }, - fieldConfig: { - defaults: { - custom: {}, - }, - overrides: [], + }, + fieldConfig: { + defaults: { + custom: {}, }, - cardOptions: { - previewModifier: (s) => { - if (s.fieldConfig?.defaults.custom?.drawStyle !== GraphDrawStyle.Bars) { - s.fieldConfig!.defaults.custom!.lineWidth = Math.max(s.fieldConfig!.defaults.custom!.lineWidth ?? 1, 2); - } - }, + overrides: [], + }, + cardOptions: { + previewModifier: (s) => { + if (s.fieldConfig?.defaults.custom?.drawStyle !== GraphDrawStyle.Bars) { + s.fieldConfig!.defaults.custom!.lineWidth = Math.max(s.fieldConfig!.defaults.custom!.lineWidth ?? 1, 2); + } }, - }); + }, + } satisfies VisualizationSuggestion); - const maxBarsCount = 100; +const areaChart = (name: string, stacking?: StackingMode) => ({ + name, + fieldConfig: { + defaults: { + custom: { + fillOpacity: 25, + ...(stacking ? { stacking: { mode: stacking, group: 'A' } } : {}), + }, + }, + overrides: [], + }, +}); - list.append({ - name: SuggestionName.LineChart, - }); +const barChart = (name: string, stacking?: StackingMode) => ({ + name, + fieldConfig: { + defaults: { + custom: { + drawStyle: GraphDrawStyle.Bars, + fillOpacity: 100, + lineWidth: 1, + gradientMode: GraphGradientMode.Hue, + ...(stacking ? { stacking: { mode: stacking, group: 'A' } } : {}), + }, + }, + overrides: [], + }, +}); - if (dataSummary.rowCountMax < 200) { - list.append({ - name: SuggestionName.LineChartSmooth, - fieldConfig: { - defaults: { - custom: { - lineInterpolation: LineInterpolation.Smooth, - }, - }, - overrides: [], - }, - }); - } +// TODO: all "gradient color scheme" suggestions have been removed. they will be re-added as part of the "styles" feature. - // Single series suggestions - if (dataSummary.numberFieldCount === 1) { - list.append({ - name: SuggestionName.AreaChart, - fieldConfig: { - defaults: { - custom: { - fillOpacity: 25, - }, - }, - overrides: [], - }, - }); +export const timeseriesSuggestionsSupplier: VisualizationSuggestionsSupplierFn = ( + dataSummary +) => { + if ( + !dataSummary.hasFieldType(FieldType.time) || + !dataSummary.hasFieldType(FieldType.number) || + dataSummary.rowCountTotal < 2 + ) { + return; + } - list.append({ - name: SuggestionName.LineChartGradientColorScheme, - fieldConfig: { - defaults: { - color: { - mode: FieldColorModeId.ContinuousGrYlRd, - }, - custom: { - gradientMode: GraphGradientMode.Scheme, - lineInterpolation: LineInterpolation.Smooth, - lineWidth: 3, - fillOpacity: 20, - }, - }, - overrides: [], - }, - }); + const score: VisualizationSuggestionScore = + dataSummary.hasDataFrameType(DataFrameType.TimeSeriesLong) || + dataSummary.hasDataFrameType(DataFrameType.TimeSeriesWide) || + dataSummary.hasDataFrameType(DataFrameType.TimeSeriesMulti) + ? VisualizationSuggestionScore.Good + : VisualizationSuggestionScore.OK; - if (dataSummary.rowCountMax < maxBarsCount) { - list.append({ - name: SuggestionName.BarChart, - fieldConfig: { - defaults: { - custom: { - drawStyle: GraphDrawStyle.Bars, - fillOpacity: 100, - lineWidth: 1, - gradientMode: GraphGradientMode.Hue, - }, - }, - overrides: [], - }, - }); + const suggestions: Array> = [ + { + name: t('timeseries.suggestions.line', 'Line chart'), + }, + ]; - list.append({ - name: SuggestionName.BarChartGradientColorScheme, - fieldConfig: { - defaults: { - color: { - mode: FieldColorModeId.ContinuousGrYlRd, - }, - custom: { - drawStyle: GraphDrawStyle.Bars, - fillOpacity: 90, - lineWidth: 1, - gradientMode: GraphGradientMode.Scheme, - }, - }, - overrides: [], - }, - }); - } - - return; - } - - // Multiple series suggestions - - list.append({ - name: SuggestionName.AreaChartStacked, + if (dataSummary.rowCountMax < MAX_ROWS_SMOOTH_CHART) { + suggestions.push({ + name: t('timeseries.suggestions.line-smooth', 'Line chart - smooth'), fieldConfig: { defaults: { custom: { - fillOpacity: 25, - stacking: { - mode: StackingMode.Normal, - group: 'A', - }, + lineInterpolation: LineInterpolation.Smooth, }, }, overrides: [], }, }); + } - list.append({ - name: SuggestionName.AreaChartStackedPercent, - fieldConfig: { - defaults: { - custom: { - fillOpacity: 25, - stacking: { - mode: StackingMode.Percent, - group: 'A', - }, - }, - }, - overrides: [], - }, - }); + // Single-series suggestions + if (dataSummary.fieldCountByType(FieldType.number) === 1) { + suggestions.push(areaChart(t('timeseries.suggestions.area', 'Area chart'))); - if (dataSummary.rowCountTotal / dataSummary.numberFieldCount < maxBarsCount) { - list.append({ - name: SuggestionName.BarChartStacked, - fieldConfig: { - defaults: { - custom: { - drawStyle: GraphDrawStyle.Bars, - fillOpacity: 100, - lineWidth: 1, - gradientMode: GraphGradientMode.Hue, - stacking: { - mode: StackingMode.Normal, - group: 'A', - }, - }, - }, - overrides: [], - }, - }); - - list.append({ - name: SuggestionName.BarChartStackedPercent, - fieldConfig: { - defaults: { - custom: { - drawStyle: GraphDrawStyle.Bars, - fillOpacity: 100, - lineWidth: 1, - gradientMode: GraphGradientMode.Hue, - stacking: { - mode: StackingMode.Percent, - group: 'A', - }, - }, - }, - overrides: [], - }, - }); + if (dataSummary.rowCountMax < MAX_BARS) { + suggestions.push(barChart(t('timeseries.suggestions.bar', 'Bar chart'))); } } -} + // Multiple series suggestions + else { + suggestions.push( + areaChart(t('timeseries.suggestions.area-stacked', 'Area chart - stacked'), StackingMode.Normal), + areaChart( + t('timeseries.suggestions.area-stacked-percentage', 'Area chart - stacked by percentage'), + StackingMode.Percent + ) + ); + + if (dataSummary.rowCountTotal / dataSummary.fieldCountByType(FieldType.number) < MAX_BARS) { + suggestions.push( + barChart(t('timeseries.suggestions.bar-stacked', 'Bar chart - stacked'), StackingMode.Normal), + barChart( + t('timeseries.suggestions.bar-stacked-percent', 'Bar chart - stacked by percentage'), + StackingMode.Percent + ) + ); + } + } + + return suggestions.map((s) => { + s.score = score; + return withDefaults(s); + }); +}; // This will try to get a suggestion that will add a long to wide conversion export function getPrepareTimeseriesSuggestion(panelId: number): PanelPluginVisualizationSuggestion | undefined { diff --git a/public/app/plugins/panel/traces/module.tsx b/public/app/plugins/panel/traces/module.tsx index fdbe35c6c91..07b72f00d53 100644 --- a/public/app/plugins/panel/traces/module.tsx +++ b/public/app/plugins/panel/traces/module.tsx @@ -1,11 +1,11 @@ import { PanelPlugin } from '@grafana/data'; import { t } from '@grafana/i18n'; +import { showDefaultSuggestion } from 'app/features/panel/suggestions/utils'; import { migrateToAdhocFilters } from '../../../features/explore/TraceView/useSearch'; import { FiltersEditor } from './FiltersEditor'; import { TracesPanel } from './TracesPanel'; -import { TracesSuggestionsSupplier } from './suggestions'; export const plugin = new PanelPlugin(TracesPanel) .setMigrationHandler((panel) => { @@ -44,4 +44,4 @@ export const plugin = new PanelPlugin(TracesPanel) category, }); }) - .setSuggestionsSupplier(new TracesSuggestionsSupplier()); + .setSuggestionsSupplier(showDefaultSuggestion((ds) => ds.hasPreferredVisualisationType('trace'))); diff --git a/public/app/plugins/panel/traces/suggestions.ts b/public/app/plugins/panel/traces/suggestions.ts deleted file mode 100644 index bda2e463927..00000000000 --- a/public/app/plugins/panel/traces/suggestions.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { VisualizationSuggestionsBuilder, VisualizationSuggestionScore } from '@grafana/data'; -import { SuggestionName } from 'app/types/suggestions'; - -export class TracesSuggestionsSupplier { - getListWithDefaults(builder: VisualizationSuggestionsBuilder) { - return builder.getListAppender<{}, {}>({ - name: SuggestionName.Trace, - pluginId: 'traces', - }); - } - - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - if (!builder.data) { - return; - } - - const dataFrame = builder.data.series[0]; - if (!dataFrame) { - return; - } - - if (builder.data.series[0].meta?.preferredVisualisationType === 'trace') { - this.getListWithDefaults(builder).append({ - name: SuggestionName.Trace, - score: VisualizationSuggestionScore.Best, - }); - } - } -} diff --git a/public/app/plugins/panel/trend/TrendPanel.tsx b/public/app/plugins/panel/trend/TrendPanel.tsx index 95ce2330283..b446c651733 100644 --- a/public/app/plugins/panel/trend/TrendPanel.tsx +++ b/public/app/plugins/panel/trend/TrendPanel.tsx @@ -1,7 +1,6 @@ import { useMemo } from 'react'; import { - isLikelyAscendingVector, DataFrame, FieldMatcherID, fieldMatchers, @@ -10,18 +9,17 @@ import { TimeRange, useDataLinksContext, } from '@grafana/data'; -import { config, PanelDataErrorView } from '@grafana/runtime'; +import { PanelDataErrorView } from '@grafana/runtime'; import { KeyboardPlugin, TooltipDisplayMode, TooltipPlugin2, usePanelContext } from '@grafana/ui'; import { TooltipHoverMode } from '@grafana/ui/internal'; import { XYFieldMatchers } from 'app/core/components/GraphNG/types'; import { preparePlotFrame } from 'app/core/components/GraphNG/utils'; import { TimeSeries } from 'app/core/components/TimeSeries/TimeSeries'; -import { findFieldIndex } from 'app/features/dimensions/utils'; import { TimeSeriesTooltip } from '../timeseries/TimeSeriesTooltip'; -import { prepareGraphableFields } from '../timeseries/utils'; import { Options } from './panelcfg.gen'; +import { prepSeries } from './utils'; export const TrendPanel = ({ data, @@ -51,49 +49,7 @@ export const TrendPanel = ({ return preparePlotFrame(frames, dimFields); }; - const info = useMemo(() => { - if (data.series.length > 1) { - return { - warning: 'Only one frame is supported, consider adding a join transformation', - frames: data.series, - }; - } - - let frames = data.series; - let xFieldIdx: number | undefined; - if (options.xField) { - xFieldIdx = findFieldIndex(options.xField, frames[0]); - if (xFieldIdx == null) { - return { - warning: 'Unable to find field: ' + options.xField, - frames: data.series, - }; - } - } else { - // first number field - // Perhaps we can/should support any ordinal rather than an error here - xFieldIdx = frames[0] ? frames[0].fields.findIndex((f) => f.type === FieldType.number) : -1; - if (xFieldIdx === -1) { - return { - warning: 'No numeric fields found for X axis', - frames, - }; - } - } - - // Make sure values are ascending - if (xFieldIdx != null) { - const field = frames[0].fields[xFieldIdx]; - if (field.type === FieldType.number && !isLikelyAscendingVector(field.values)) { - return { - warning: `Values must be in ascending order`, - frames, - }; - } - } - - return { frames: prepareGraphableFields(frames, config.theme2, undefined, xFieldIdx) }; - }, [data.series, options.xField]); + const info = useMemo(() => prepSeries(data.series, options.xField), [data.series, options.xField]); if (info.warning || !info.frames) { return ( diff --git a/public/app/plugins/panel/trend/module.tsx b/public/app/plugins/panel/trend/module.tsx index ec823e00644..67434a8c542 100644 --- a/public/app/plugins/panel/trend/module.tsx +++ b/public/app/plugins/panel/trend/module.tsx @@ -1,13 +1,14 @@ -import { Field, FieldType, PanelPlugin } from '@grafana/data'; +import { Field, FieldType, PanelPlugin, VisualizationSuggestionScore } from '@grafana/data'; import { t } from '@grafana/i18n'; -import { commonOptionsBuilder } from '@grafana/ui'; +import { GraphDrawStyle } from '@grafana/schema'; +import { commonOptionsBuilder, LegendDisplayMode } from '@grafana/ui'; import { optsWithHideZeros } from '@grafana/ui/internal'; import { defaultGraphConfig, getGraphFieldConfig } from '../timeseries/config'; import { TrendPanel } from './TrendPanel'; import { FieldConfig, Options } from './panelcfg.gen'; -import { TrendSuggestionsSupplier } from './suggestions'; +import { prepSeries } from './utils'; export const plugin = new PanelPlugin(TrendPanel) .useFieldConfig(getGraphFieldConfig(defaultGraphConfig, false)) @@ -29,5 +30,47 @@ export const plugin = new PanelPlugin(TrendPanel) commonOptionsBuilder.addTooltipOptions(builder, false, true, optsWithHideZeros); commonOptionsBuilder.addLegendOptions(builder); }) - .setSuggestionsSupplier(new TrendSuggestionsSupplier()); + .setSuggestionsSupplier((ds) => { + if ( + !ds.rawFrames || + ds.fieldCountByType(FieldType.number) < 2 || + ds.rowCountTotal < 2 || + ds.rowCountTotal < 2 || + ds.frameCount > 1 + ) { + return; + } + + const info = prepSeries(ds.rawFrames); + if (info.warning || !info.frames) { + return; + } + + return [ + { + score: VisualizationSuggestionScore.Good, + options: { + legend: { + calcs: [], + displayMode: LegendDisplayMode.Hidden, + placement: 'right', + showLegend: false, + }, + }, + fieldConfig: { + defaults: { + custom: {}, + }, + overrides: [], + }, + cardOptions: { + previewModifier: (s) => { + if (s.fieldConfig?.defaults.custom?.drawStyle !== GraphDrawStyle.Bars) { + s.fieldConfig!.defaults.custom!.lineWidth = Math.max(s.fieldConfig!.defaults.custom!.lineWidth ?? 1, 2); + } + }, + }, + }, + ]; + }); //.setDataSupport({ annotations: true, alertStates: true }); diff --git a/public/app/plugins/panel/trend/suggestions.ts b/public/app/plugins/panel/trend/suggestions.ts deleted file mode 100644 index c135c5df30e..00000000000 --- a/public/app/plugins/panel/trend/suggestions.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { VisualizationSuggestionsBuilder } from '@grafana/data'; -import { GraphDrawStyle, GraphFieldConfig, LegendDisplayMode } from '@grafana/schema'; -import { SuggestionName } from 'app/types/suggestions'; - -import { Options } from './panelcfg.gen'; - -export class TrendSuggestionsSupplier { - getSuggestionsForData(builder: VisualizationSuggestionsBuilder) { - const { dataSummary } = builder; - - if (dataSummary.numberFieldCount < 2 || dataSummary.rowCountTotal < 2 || dataSummary.rowCountTotal < 2) { - return; - } - - // Super basic - const list = builder.getListAppender({ - name: SuggestionName.LineChart, - pluginId: 'trend', - options: { - legend: { - calcs: [], - displayMode: LegendDisplayMode.Hidden, - placement: 'right', - showLegend: false, - }, - }, - fieldConfig: { - defaults: { - custom: {}, - }, - overrides: [], - }, - cardOptions: { - previewModifier: (s) => { - if (s.fieldConfig?.defaults.custom?.drawStyle !== GraphDrawStyle.Bars) { - s.fieldConfig!.defaults.custom!.lineWidth = Math.max(s.fieldConfig!.defaults.custom!.lineWidth ?? 1, 2); - } - }, - }, - }); - return list; - } -} diff --git a/public/app/plugins/panel/trend/utils.ts b/public/app/plugins/panel/trend/utils.ts new file mode 100644 index 00000000000..17a02fd5049 --- /dev/null +++ b/public/app/plugins/panel/trend/utils.ts @@ -0,0 +1,48 @@ +import { DataFrame, FieldType, isLikelyAscendingVector } from '@grafana/data'; +import config from 'app/core/config'; +import { findFieldIndex } from 'app/features/dimensions/utils'; + +import { prepareGraphableFields } from '../timeseries/utils'; + +export function prepSeries(frames: DataFrame[], xField?: string): { warning?: string; frames: DataFrame[] | null } { + if (frames.length > 1) { + return { + warning: 'Only one frame is supported, consider adding a join transformation', + frames: frames, + }; + } + + let xFieldIdx: number | undefined; + if (xField) { + xFieldIdx = findFieldIndex(xField, frames[0]); + if (xFieldIdx == null) { + return { + warning: 'Unable to find field: ' + xField, + frames: frames, + }; + } + } else { + // first number field + // Perhaps we can/should support any ordinal rather than an error here + xFieldIdx = frames[0] ? frames[0].fields.findIndex((f) => f.type === FieldType.number) : -1; + if (xFieldIdx === -1) { + return { + warning: 'No numeric fields found for X axis', + frames, + }; + } + } + + // Make sure values are ascending + if (xFieldIdx != null) { + const field = frames[0].fields[xFieldIdx]; + if (field.type === FieldType.number && !isLikelyAscendingVector(field.values)) { + return { + warning: `Values must be in ascending order`, + frames, + }; + } + } + + return { frames: prepareGraphableFields(frames, config.theme2, undefined, xFieldIdx) }; +} diff --git a/public/app/types/suggestions.ts b/public/app/types/suggestions.ts deleted file mode 100644 index aac0dc1bd3c..00000000000 --- a/public/app/types/suggestions.ts +++ /dev/null @@ -1,34 +0,0 @@ -export enum SuggestionName { - LineChart = 'Line chart', - LineChartSmooth = 'Line chart smooth', - LineChartGradientColorScheme = 'Line chart with gradient color scheme', - AreaChart = 'Area chart', - AreaChartStacked = 'Area chart stacked', - AreaChartStackedPercent = 'Area chart 100% stacked', - BarChart = 'Bar chart', - BarChartGradientColorScheme = 'Bar chart with gradient color scheme', - BarChartStacked = 'Bar chart stacked', - BarChartStackedPercent = 'Bar chart 100% stacked', - BarChartHorizontal = 'Bar chart horizontal', - BarChartHorizontalStacked = 'Bar chart horizontal stacked', - BarChartHorizontalStackedPercent = 'Bar chart horizontal 100% stacked', - Candlestick = 'Candlestick', - PieChart = 'Pie chart', - PieChartDonut = 'Pie chart donut', - Stat = 'Stat', - StatColoredBackground = 'Stat colored background', - Gauge = 'Gauge', - GaugeCircular = 'Circular gauge', - GaugeNoThresholds = 'Gauge no thresholds', - BarGaugeBasic = 'Bar gauge basic', - BarGaugeLCD = 'Bar gauge LCD', - Table = 'Table', - StateTimeline = 'State timeline', - StatusHistory = 'Status history', - TextPanel = 'Text', - DashboardList = 'Dashboard list', - Logs = 'Logs', - FlameGraph = 'Flame graph', - Trace = 'Trace', - NodeGraph = 'Node graph', -} diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 9f8b19218de..aa73bf3cd22 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -3477,6 +3477,14 @@ "label-negative-y": "Negative Y" } }, + "suggestions": { + "horizontal": "Horizontal bar chart", + "hz-stacked": "Horizontal bar chart - stacked", + "hz-stacked-percent": "Horizontal bar chart - stacked by percentage", + "vert-stacked": "Bar chart - stacked", + "vert-stacked-percent": "Bar chart - stacked by percentage", + "vertical": "Bar chart" + }, "tick-spacing-editor": { "content-require-space-from-the-right-side": "Require space from the right side", "gaps-options": { @@ -3521,6 +3529,10 @@ }, "name-show-unfilled-area": "Show unfilled area", "name-value-display": "Value display", + "suggestions": { + "basic": "Bar gauge", + "lcd": "Bar gauge - LCD" + }, "value-display-options": { "label-hidden": "Hidden", "label-text-color": "Text color", @@ -7849,6 +7861,7 @@ "suggestions": { "arc": "Gauge", "circular": "Circular gauge", + "no-thresholds": "Gauge - no thresholds", "style": { "circular": "Glowing", "simple": "Simple" @@ -11103,6 +11116,10 @@ "pie-chart-type-options": { "label-donut": "Donut", "label-pie": "Pie" + }, + "suggestions": { + "donut": "Donut chart", + "pie": "Pie chart" } }, "playlist": { @@ -13051,6 +13068,12 @@ "label-same-as-value": "Same as Value", "label-standard": "Standard" }, + "suggestions": { + "stat-color-background": "Stat - color background", + "stat-discrete-values": "Stat - discrete values", + "stat-discrete-values-color-background": "Stat - discrete values - color background", + "stat-single-string": "Stat - single string" + }, "text-alignment-options": { "label-auto": "Auto", "label-center": "Center" @@ -13490,6 +13513,16 @@ "label-threshold": "Threshold" } }, + "suggestions": { + "area": "Area chart", + "area-stacked": "Area chart - stacked", + "area-stacked-percentage": "Area chart - stacked by percentage", + "bar": "Bar chart", + "bar-stacked": "Bar chart - stacked", + "bar-stacked-percent": "Bar chart - stacked by percentage", + "line": "Line chart", + "line-smooth": "Line chart - smooth" + }, "timezones-editor": { "tooltip-add-timezone": "Add timezone", "tooltip-remove-timezone": "Remove timezone" From c29ed31c7ab977833642fcfe3aebd987dd09675f Mon Sep 17 00:00:00 2001 From: Tito Lins Date: Wed, 26 Nov 2025 17:59:22 +0100 Subject: [PATCH 02/14] alerting: set model refID if missing/mismatch (#114441) --- pkg/services/ngalert/models/alert_query.go | 20 +++++++++++++++++++ pkg/tests/api/alerting/api_prometheus_test.go | 14 ++++++------- pkg/tests/api/alerting/api_ruler_test.go | 14 ++++++++++++- .../test-data/rulegroup-1-export.json | 2 ++ .../test-data/rulegroup-2-export.json | 1 + .../test-data/rulegroup-3-export.json | 2 ++ 6 files changed, 45 insertions(+), 8 deletions(-) diff --git a/pkg/services/ngalert/models/alert_query.go b/pkg/services/ngalert/models/alert_query.go index 9d25d8cfa71..f86925eaa9b 100644 --- a/pkg/services/ngalert/models/alert_query.go +++ b/pkg/services/ngalert/models/alert_query.go @@ -165,6 +165,21 @@ func (aq *AlertQuery) setMaxDatapoints() error { return nil } +// setRefID sets the model refId if it's missing or invalid +func (aq *AlertQuery) setRefID() error { + if aq.modelProps == nil { + err := aq.setModelProps() + if err != nil { + return err + } + } + + if refID, ok := aq.modelProps["refId"].(string); !ok || refID != aq.RefID { + aq.modelProps["refId"] = aq.RefID + } + return nil +} + func (aq *AlertQuery) GetMaxDatapoints() (int64, error) { err := aq.setMaxDatapoints() if err != nil { @@ -256,6 +271,11 @@ func (aq *AlertQuery) GetModel() ([]byte, error) { return nil, err } + err = aq.setRefID() + if err != nil { + return nil, err + } + err = aq.setIntervalMS() if err != nil { return nil, err diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index 23681381fc9..61bc3195857 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -252,7 +252,7 @@ func TestIntegrationPrometheusRules(t *testing.T) { "rules": [{ "state": "inactive", "name": "AlwaysFiring", - "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"type\":\"math\"}}]", + "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"refId\":\"A\",\"type\":\"math\"}}]", "duration": 10, "folderUid": "default", "uid": "%s", @@ -270,7 +270,7 @@ func TestIntegrationPrometheusRules(t *testing.T) { }, { "state": "inactive", "name": "AlwaysFiringButSilenced", - "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"type\":\"math\"}}]", + "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"refId\":\"A\",\"type\":\"math\"}}]", "folderUid": "default", "uid": "%s", "health": "ok", @@ -317,7 +317,7 @@ func TestIntegrationPrometheusRules(t *testing.T) { "rules": [{ "state": "inactive", "name": "AlwaysFiring", - "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"type\":\"math\"}}]", + "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"refId\":\"A\",\"type\":\"math\"}}]", "duration": 10, "folderUid": "default", "uid": "%s", @@ -335,7 +335,7 @@ func TestIntegrationPrometheusRules(t *testing.T) { }, { "state": "inactive", "name": "AlwaysFiringButSilenced", - "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"type\":\"math\"}}]", + "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"refId\":\"A\",\"type\":\"math\"}}]", "folderUid": "default", "uid": "%s", "health": "ok", @@ -639,7 +639,7 @@ func TestIntegrationPrometheusRulesFilterByDashboard(t *testing.T) { "name": "AlwaysFiring", "uid": "%s", "folderUid": "default", - "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"type\":\"math\"}}]", + "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"refId\":\"A\",\"type\":\"math\"}}]", "duration": 10, "keepFiringFor": 15, "annotations": { @@ -656,7 +656,7 @@ func TestIntegrationPrometheusRulesFilterByDashboard(t *testing.T) { "name": "AlwaysFiringButSilenced", "uid": "%s", "folderUid": "default", - "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"type\":\"math\"}}]", + "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"refId\":\"A\",\"type\":\"math\"}}]", "health": "ok", "isPaused": false, "type": "alerting", @@ -688,7 +688,7 @@ func TestIntegrationPrometheusRulesFilterByDashboard(t *testing.T) { "name": "AlwaysFiring", "uid": "%s", "folderUid": "default", - "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"type\":\"math\"}}]", + "query": "[{\"refId\":\"A\",\"queryType\":\"\",\"relativeTimeRange\":{\"from\":18000,\"to\":10800},\"datasourceUid\":\"__expr__\",\"model\":{\"expression\":\"2 + 3 \\u003e 1\",\"intervalMs\":1000,\"maxDataPoints\":43200,\"refId\":\"A\",\"type\":\"math\"}}]", "duration": 10, "keepFiringFor": 15, "annotations": { diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index 18eb018b336..9e8855d8f19 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -1166,6 +1166,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "expression": "2 + 3 \u003e 1", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } }], @@ -1209,6 +1210,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "expression": "2 + 3 \u003e 1", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } }], @@ -1264,6 +1266,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "expression": "2 + 3 \u003e 1", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } }], @@ -1610,7 +1613,7 @@ func TestIntegrationRuleCreate(t *testing.T) { To: apimodels.Duration(15 * time.Minute), }, DatasourceUID: expr.DatasourceUID, - Model: json.RawMessage(`{"expression":"1","intervalMs":1000,"maxDataPoints":43200,"type":"math"}`), + Model: json.RawMessage(`{"expression":"1","intervalMs":1000,"maxDataPoints":43200,"refId":"A","type":"math"}`), }, }, UpdatedBy: &apimodels.UserInfo{ @@ -2681,6 +2684,7 @@ func TestIntegrationQuota(t *testing.T) { "expression":"2 + 4 \u003E 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } @@ -2798,6 +2802,7 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) { "expression": "2 + 3 > 1", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } } @@ -3285,6 +3290,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "expression":"2 + 3 \u003e 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } @@ -3331,6 +3337,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "expression":"2 + 3 \u003e 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } @@ -3683,6 +3690,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "expression":"2 + 3 \u003e 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } @@ -3729,6 +3737,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "expression":"2 + 3 \u003e 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } @@ -3872,6 +3881,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "expression":"2 + 3 \u003C 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } @@ -3995,6 +4005,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "expression":"2 + 3 \u003C 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } @@ -4093,6 +4104,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "expression":"2 + 3 \u003C 1", "intervalMs":1000, "maxDataPoints":43200, + "refId":"A", "type":"math" } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json index dbf2ff417b0..18d8b8cea40 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json @@ -23,6 +23,7 @@ "expression": "0 \u003e 0", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } } @@ -55,6 +56,7 @@ "expression": "0 == 0", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json index 5f85d830260..43428689eb1 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json @@ -23,6 +23,7 @@ "expression": "0/0", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json index 5a1ba9e0eac..76eda60f08d 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json @@ -23,6 +23,7 @@ "expression": "0 \u003e 0", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } } @@ -54,6 +55,7 @@ "expression": "0 == 0", "intervalMs": 1000, "maxDataPoints": 43200, + "refId": "A", "type": "math" } } From d732bb2751175b3fd2c1288294dd98d80316dc59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 26 Nov 2025 19:26:53 +0100 Subject: [PATCH 03/14] PanelChrome: Enable new panel padding by default (#114492) --- .../configure-grafana/feature-toggles/index.md | 1 + .../grafana-data/src/types/featureToggles.gen.ts | 2 +- pkg/services/featuremgmt/registry.go | 6 +++--- pkg/services/featuremgmt/toggles_gen.csv | 2 +- pkg/services/featuremgmt/toggles_gen.go | 4 ---- pkg/services/featuremgmt/toggles_gen.json | 14 ++++++++------ 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index c05ecd45bd9..7ca66464680 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -98,6 +98,7 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `azureResourcePickerUpdates` | Enables the updated Azure Monitor resource picker | | `newVizSuggestions` | Enable new visualization suggestions | | `preventPanelChromeOverflow` | Restrict PanelChrome contents with overflow: hidden; | +| `newPanelPadding` | Increases panel padding globally | | `transformationsEmptyPlaceholder` | Show transformation quick-start cards in empty transformations state | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 0d20e44b19e..40f2abe4d73 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -1150,7 +1150,7 @@ export interface FeatureToggles { pluginStoreServiceLoading?: boolean; /** * Increases panel padding globally - * @default false + * @default true */ newPanelPadding?: boolean; /** diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 65f94c24eac..91b59bf711d 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1895,10 +1895,10 @@ var ( { Name: "newPanelPadding", Description: "Increases panel padding globally", - Stage: FeatureStageExperimental, - FrontendOnly: false, + Stage: FeatureStagePublicPreview, + FrontendOnly: true, Owner: grafanaDashboardsSquad, - Expression: "false", + Expression: "true", }, { Name: "onlyStoreActionSets", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 17fdf27f33d..f161087ecb1 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -257,7 +257,7 @@ newVizSuggestions,preview,@grafana/dataviz-squad,false,false,true preventPanelChromeOverflow,preview,@grafana/grafana-frontend-platform,false,false,true jaegerEnableGrpcEndpoint,experimental,@grafana/oss-big-tent,false,false,false pluginStoreServiceLoading,experimental,@grafana/plugins-platform-backend,false,false,false -newPanelPadding,experimental,@grafana/dashboards-squad,false,false,false +newPanelPadding,preview,@grafana/dashboards-squad,false,false,true onlyStoreActionSets,GA,@grafana/identity-access-team,false,false,false panelTimeSettings,experimental,@grafana/dashboards-squad,false,false,false kubernetesAnnotations,experimental,@grafana/grafana-backend-services-squad,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index b88204db708..1217d1f5a28 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -742,10 +742,6 @@ const ( // Load plugins on store service startup instead of wire provider, and call RegisterFixedRoles after all plugins are loaded FlagPluginStoreServiceLoading = "pluginStoreServiceLoading" - // FlagNewPanelPadding - // Increases panel padding globally - FlagNewPanelPadding = "newPanelPadding" - // FlagOnlyStoreActionSets // When storing dashboard and folder resource permissions, only store action sets and not the full list of underlying permission FlagOnlyStoreActionSets = "onlyStoreActionSets" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 1093b643232..8dee7f8206b 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -551,7 +551,6 @@ "description": "Enables the UI to use rules backend-side filters 100% compatible with the frontend filters", "stage": "experimental", "codeowner": "@grafana/alerting-squad", - "hideFromAdminPage": true, "hideFromDocs": true } }, @@ -565,7 +564,6 @@ "description": "Enables the UI to use rules backend-side filters 100% compatible with the frontend filters", "stage": "experimental", "codeowner": "@grafana/alerting-squad", - "hideFromAdminPage": true, "hideFromDocs": true } }, @@ -2361,14 +2359,18 @@ { "metadata": { "name": "newPanelPadding", - "resourceVersion": "1763734583253", - "creationTimestamp": "2025-11-12T15:40:46Z" + "resourceVersion": "1764168915089", + "creationTimestamp": "2025-11-12T15:40:46Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-11-26 14:55:15.089551 +0000 UTC" + } }, "spec": { "description": "Increases panel padding globally", - "stage": "experimental", + "stage": "preview", "codeowner": "@grafana/dashboards-squad", - "expression": "false" + "frontend": true, + "expression": "true" } }, { From 7eb467e561ef7b01fb259ba866f5a1e9c956163c Mon Sep 17 00:00:00 2001 From: maicon Date: Wed, 26 Nov 2025 16:24:34 -0300 Subject: [PATCH 04/14] provisioning: acquire server lock before provisioning dashboards+folders (#114488) * provisioning: acquire server lock before provisioning dashboards+folders Signed-off-by: Maicon Costa --------- Signed-off-by: Maicon Costa --- pkg/server/wire_gen.go | 4 +- .../provisioning/dashboards/dashboard.go | 67 ++++++++++++++----- pkg/services/provisioning/provisioning.go | 6 +- .../provisioning/provisioning_test.go | 4 +- pkg/setting/setting.go | 13 ++++ 5 files changed, 75 insertions(+), 19 deletions(-) diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 64ffd48ca9f..75341e77f78 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -668,7 +668,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api azurePromMigrationService := promtypemigration.ProvideAzurePromMigrationService(service15, inMemory, repoManager, pluginInstaller, cfg) amazonPromMigrationService := promtypemigration.ProvideAmazonPromMigrationService(service15, inMemory, repoManager, pluginInstaller, cfg) promTypeMigrationProviderImpl := promtypemigration.ProvidePromTypeMigrationProvider(serverLockService, featureToggles, azurePromMigrationService, amazonPromMigrationService) - provisioningServiceImpl, err := provisioning.ProvideService(accessControl, cfg, sqlStore, pluginstoreService, dBstore, serviceService, notificationService, dashboardProvisioningService, service15, correlationsService, dashboardService, folderimplService, service13, searchService, quotaService, secretsService, orgService, receiverPermissionsService, tracingService, dualwriteService, promTypeMigrationProviderImpl) + provisioningServiceImpl, err := provisioning.ProvideService(accessControl, cfg, sqlStore, pluginstoreService, dBstore, serviceService, notificationService, dashboardProvisioningService, service15, correlationsService, dashboardService, folderimplService, service13, searchService, quotaService, secretsService, orgService, receiverPermissionsService, tracingService, dualwriteService, promTypeMigrationProviderImpl, serverLockService) if err != nil { return nil, err } @@ -1312,7 +1312,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac azurePromMigrationService := promtypemigration.ProvideAzurePromMigrationService(service15, inMemory, repoManager, pluginInstaller, cfg) amazonPromMigrationService := promtypemigration.ProvideAmazonPromMigrationService(service15, inMemory, repoManager, pluginInstaller, cfg) promTypeMigrationProviderImpl := promtypemigration.ProvidePromTypeMigrationProvider(serverLockService, featureToggles, azurePromMigrationService, amazonPromMigrationService) - provisioningServiceImpl, err := provisioning.ProvideService(accessControl, cfg, sqlStore, pluginstoreService, dBstore, serviceService, notificationService, dashboardProvisioningService, service15, correlationsService, dashboardService, folderimplService, service13, searchService, quotaService, secretsService, orgService, receiverPermissionsService, tracingService, dualwriteService, promTypeMigrationProviderImpl) + provisioningServiceImpl, err := provisioning.ProvideService(accessControl, cfg, sqlStore, pluginstoreService, dBstore, serviceService, notificationService, dashboardProvisioningService, service15, correlationsService, dashboardService, folderimplService, service13, searchService, quotaService, secretsService, orgService, receiverPermissionsService, tracingService, dualwriteService, promTypeMigrationProviderImpl, serverLockService) if err != nil { return nil, err } diff --git a/pkg/services/provisioning/dashboards/dashboard.go b/pkg/services/provisioning/dashboards/dashboard.go index e0c24c6ab07..72b980e4198 100644 --- a/pkg/services/provisioning/dashboards/dashboard.go +++ b/pkg/services/provisioning/dashboards/dashboard.go @@ -2,6 +2,7 @@ package dashboards import ( "context" + "errors" "fmt" "os" "time" @@ -9,10 +10,12 @@ import ( dashboardV1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" folderV1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/provisioning/utils" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/legacysql/dualwrite" ) @@ -28,7 +31,7 @@ type DashboardProvisioner interface { } // DashboardProvisionerFactory creates DashboardProvisioners based on input -type DashboardProvisionerFactory func(context.Context, string, dashboards.DashboardProvisioningService, org.Service, utils.DashboardStore, folder.Service, dualwrite.Service) (DashboardProvisioner, error) +type DashboardProvisionerFactory func(context.Context, string, dashboards.DashboardProvisioningService, *setting.Cfg, org.Service, utils.DashboardStore, folder.Service, dualwrite.Service, *serverlock.ServerLockService) (DashboardProvisioner, error) // Provisioner is responsible for syncing dashboard from disk to Grafana's database. type Provisioner struct { @@ -38,6 +41,8 @@ type Provisioner struct { duplicateValidator duplicateValidator provisioner dashboards.DashboardProvisioningService dual dualwrite.Service + serverLock *serverlock.ServerLockService + cfg *setting.Cfg } func (provider *Provisioner) HasDashboardSources() bool { @@ -45,7 +50,7 @@ func (provider *Provisioner) HasDashboardSources() bool { } // New returns a new DashboardProvisioner -func New(ctx context.Context, configDirectory string, provisioner dashboards.DashboardProvisioningService, orgService org.Service, dashboardStore utils.DashboardStore, folderService folder.Service, dual dualwrite.Service) (DashboardProvisioner, error) { +func New(ctx context.Context, configDirectory string, provisioner dashboards.DashboardProvisioningService, cfg *setting.Cfg, orgService org.Service, dashboardStore utils.DashboardStore, folderService folder.Service, dual dualwrite.Service, serverLockService *serverlock.ServerLockService) (DashboardProvisioner, error) { logger := log.New("provisioning.dashboard") cfgReader := &configReader{path: configDirectory, log: logger, orgExists: utils.NewOrgExistsChecker(orgService)} configs, err := cfgReader.readConfig(ctx) @@ -78,6 +83,8 @@ func New(ctx context.Context, configDirectory string, provisioner dashboards.Das duplicateValidator: newDuplicateValidator(logger, fileReaders), provisioner: provisioner, dual: dual, + serverLock: serverLockService, + cfg: cfg, } return d, nil @@ -95,23 +102,53 @@ func (provider *Provisioner) Provision(ctx context.Context) error { } } - provider.log.Info("starting to provision dashboards") + var errProvisioning error - for _, reader := range provider.fileReaders { - if err := reader.walkDisk(ctx); err != nil { - if os.IsNotExist(err) { - // don't stop the provisioning service in case the folder is missing. The folder can appear after the startup - provider.log.Warn("Failed to provision config", "name", reader.Cfg.Name, "error", err) - return nil - } - - return fmt.Errorf("failed to provision config %v: %w", reader.Cfg.Name, err) + // retry obtaining the lock for 20 attempts + retryOpt := func(attempts int) error { + if attempts < 20 { + return nil } + return errors.New("retries exhausted") } - provider.duplicateValidator.validate() - provider.log.Info("finished to provision dashboards") - return nil + lockTimeConfig := serverlock.LockTimeConfig{ + // if a replica crashes while holding the lock, other replicas can obtain the + // lock after this duration (15s default value, might be configured via config file) + MaxInterval: time.Duration(provider.cfg.ClassicProvisioningDashboardsServerLockMaxIntervalSeconds) * time.Second, + + // wait beetween 100ms and 1s before retrying to obtain the lock (default values, might be configured via config file) + MinWait: time.Duration(provider.cfg.ClassicProvisioningDashboardsServerLockMinWaitMs) * time.Millisecond, + MaxWait: time.Duration(provider.cfg.ClassicProvisioningDashboardsServerLockMaxWaitMs) * time.Millisecond, + } + + // this means that if we fail to obtain the lock after ~10 seconds, we return an error + lockErr := provider.serverLock.LockExecuteAndReleaseWithRetries(ctx, "provisioning_dashboards", lockTimeConfig, func(ctx context.Context) { + provider.log.Info("starting to provision dashboards") + + for _, reader := range provider.fileReaders { + if err := reader.walkDisk(ctx); err != nil { + if os.IsNotExist(err) { + // don't stop the provisioning service in case the folder is missing. The folder can appear after the startup + provider.log.Warn("Failed to provision config", "name", reader.Cfg.Name, "error", err) + return + } + + errProvisioning = fmt.Errorf("failed to provision config %v: %w", reader.Cfg.Name, err) + return + } + } + + provider.duplicateValidator.validate() + provider.log.Info("finished to provision dashboards") + }, retryOpt) + + if lockErr != nil { + provider.log.Error("Failed to obtain dashboard provisioning lock", "error", lockErr) + return lockErr + } + + return errProvisioning } // CleanUpOrphanedDashboards deletes provisioned dashboards missing a linked reader. diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index c79a9625d50..bc7c53e0cb2 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/dskit/services" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -64,6 +65,7 @@ func ProvideService( tracer tracing.Tracer, dual dualwrite.Service, promTypeMigrationProvider promtypemigration.PromTypeMigrationProvider, + serverLockService *serverlock.ServerLockService, ) (*ProvisioningServiceImpl, error) { s := &ProvisioningServiceImpl{ Cfg: cfg, @@ -92,6 +94,7 @@ func ProvideService( tracer: tracer, migratePrometheusType: promTypeMigrationProvider.Run, dual: dual, + serverLock: serverLockService, } s.NamedService = services.NewBasicService(s.starting, s.running, nil).WithName(ServiceName) @@ -166,7 +169,7 @@ func (ps *ProvisioningServiceImpl) running(ctx context.Context) error { func (ps *ProvisioningServiceImpl) setDashboardProvisioner() error { dashboardPath := filepath.Join(ps.Cfg.ProvisioningPath, "dashboards") - dashProvisioner, err := ps.newDashboardProvisioner(context.Background(), dashboardPath, ps.dashboardProvisioningService, ps.orgService, ps.dashboardService, ps.folderService, ps.dual) + dashProvisioner, err := ps.newDashboardProvisioner(context.Background(), dashboardPath, ps.dashboardProvisioningService, ps.Cfg, ps.orgService, ps.dashboardService, ps.folderService, ps.dual, ps.serverLock) if err != nil { return fmt.Errorf("%v: %w", "Failed to create provisioner", err) } @@ -242,6 +245,7 @@ type ProvisioningServiceImpl struct { resourcePermissions accesscontrol.ReceiverPermissionsService tracer tracing.Tracer dual dualwrite.Service + serverLock *serverlock.ServerLockService migratePrometheusType func(context.Context) error } diff --git a/pkg/services/provisioning/provisioning_test.go b/pkg/services/provisioning/provisioning_test.go index 8b513ba321e..b94d7beac37 100644 --- a/pkg/services/provisioning/provisioning_test.go +++ b/pkg/services/provisioning/provisioning_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/serverlock" dashboardstore "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" @@ -20,6 +21,7 @@ import ( "github.com/grafana/grafana/pkg/services/provisioning/datasources" "github.com/grafana/grafana/pkg/services/provisioning/utils" "github.com/grafana/grafana/pkg/services/searchV2" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/legacysql/dualwrite" ) @@ -160,7 +162,7 @@ func setup(t *testing.T) *serviceTestStruct { searchStub := searchV2.NewStubSearchService() service, err := newProvisioningServiceImpl( - func(context.Context, string, dashboardstore.DashboardProvisioningService, org.Service, utils.DashboardStore, folder.Service, dualwrite.Service) (dashboards.DashboardProvisioner, error) { + func(context.Context, string, dashboardstore.DashboardProvisioningService, *setting.Cfg, org.Service, utils.DashboardStore, folder.Service, dualwrite.Service, *serverlock.ServerLockService) (dashboards.DashboardProvisioner, error) { serviceTest.dashboardProvisionerInstantiations++ return serviceTest.mock, nil }, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index a8b48c67f29..470b6910751 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -150,6 +150,11 @@ type Cfg struct { PluginsPath string EnterpriseLicensePath string + // Classic Provisioning settings + ClassicProvisioningDashboardsServerLockMaxIntervalSeconds int64 + ClassicProvisioningDashboardsServerLockMinWaitMs int64 + ClassicProvisioningDashboardsServerLockMaxWaitMs int64 + // SMTP email settings Smtp SmtpSettings @@ -1221,6 +1226,8 @@ func (cfg *Cfg) parseINIFile(iniFile *ini.File) error { return err } + cfg.readClassicProvisioningSettings(iniFile) + // read dashboard settings dashboards := iniFile.Section("dashboards") cfg.DashboardVersionsToKeep = dashboards.Key("versions_to_keep").MustInt(20) @@ -2107,6 +2114,12 @@ func (cfg *Cfg) readLiveSettings(iniFile *ini.File) error { return nil } +func (cfg *Cfg) readClassicProvisioningSettings(iniFile *ini.File) { + cfg.ClassicProvisioningDashboardsServerLockMinWaitMs = iniFile.Section("classic_provisioning").Key("dashboards_server_lock_min_wait_ms").MustInt64(100) + cfg.ClassicProvisioningDashboardsServerLockMaxWaitMs = iniFile.Section("classic_provisioning").Key("dashboards_server_lock_max_wait_ms").MustInt64(1000) + cfg.ClassicProvisioningDashboardsServerLockMaxIntervalSeconds = iniFile.Section("classic_provisioning").Key("dashboards_server_lock_max_interval_seconds").MustInt64(15) +} + func (cfg *Cfg) readProvisioningSettings(iniFile *ini.File) error { provisioning := valueAsString(iniFile.Section("paths"), "provisioning", "") cfg.ProvisioningPath = makeAbsolute(provisioning, cfg.HomePath) From b08e7bc3736d9204d1f4d48d49b3315754a69380 Mon Sep 17 00:00:00 2001 From: Jesse David Peterson Date: Wed, 26 Nov 2025 16:32:37 -0400 Subject: [PATCH 05/14] TimePicker: Show new shortcut for zoom out when experimental flag toggled on (#114506) * fix(time-picker): show new shortcut for zoom out when flag toggled on * chore(i18n): extract translations --- .../DateTimePickers/TimeRangePicker.test.tsx | 54 ++++++++++++++++++- .../DateTimePickers/TimeRangePicker.tsx | 24 ++++++--- public/locales/en-US/grafana.json | 3 +- 3 files changed, 72 insertions(+), 9 deletions(-) diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.test.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.test.tsx index c7399e65a4f..51d24c5bc8e 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.test.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.test.tsx @@ -1,7 +1,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { dateTime, makeTimeRange, TimeRange } from '@grafana/data'; +import { dateTime, makeTimeRange, TimeRange, BootData } from '@grafana/data'; import { selectors as e2eSelectors } from '@grafana/e2e-selectors'; import { TimeRangeProvider } from './TimeRangeContext'; @@ -152,6 +152,58 @@ it('does not submit wrapping forms', async () => { expect(onSubmit).not.toHaveBeenCalled(); }); +it('shows CTRL+Z in zoom out tooltip when feature flag is disabled', async () => { + window.grafanaBootData = { + settings: { + featureToggles: { + newTimeRangeZoomShortcuts: false, + }, + }, + } as BootData; + + render( + {}} + onChange={(value) => {}} + value={value} + onMoveBackward={() => {}} + onMoveForward={() => {}} + onZoom={() => {}} + /> + ); + + const zoomButton = screen.getByLabelText('Zoom out time range'); + await userEvent.hover(zoomButton); + + expect(await screen.findByText(/CTRL\+Z/)).toBeInTheDocument(); +}); + +it('shows t - in zoom out tooltip when feature flag is enabled', async () => { + window.grafanaBootData = { + settings: { + featureToggles: { + newTimeRangeZoomShortcuts: true, + }, + }, + } as BootData; + + render( + {}} + onChange={(value) => {}} + value={value} + onMoveBackward={() => {}} + onMoveForward={() => {}} + onZoom={() => {}} + /> + ); + + const zoomButton = screen.getByLabelText('Zoom out time range'); + await userEvent.hover(zoomButton); + + expect(await screen.findByText(/t -/)).toBeInTheDocument(); +}); + describe('TimePickerTooltip', () => { beforeAll(() => { const mockIntl = { diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.tsx index b26a6e4c36d..0719693e625 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker.tsx @@ -19,6 +19,7 @@ import { selectors } from '@grafana/e2e-selectors'; import { t, Trans } from '@grafana/i18n'; import { useStyles2 } from '../../themes/ThemeContext'; +import { getFeatureToggle } from '../../utils/featureToggle'; import { ButtonGroup } from '../Button/ButtonGroup'; import { getModalStyles } from '../Modal/getModalStyles'; import { getPortalContainer } from '../Portal/Portal'; @@ -243,13 +244,22 @@ export function TimeRangePicker(props: TimeRangePickerProps) { TimeRangePicker.displayName = 'TimeRangePicker'; -const ZoomOutTooltip = () => ( - <> - - Time range zoom out
CTRL+Z - - -); +const ZoomOutTooltip = () => { + const newShortcuts = getFeatureToggle('newTimeRangeZoomShortcuts'); + return ( + <> + {newShortcuts ? ( + + Time range zoom out
t - + + ) : ( + + Time range zoom out
CTRL+Z + + )} + + ); +}; export const TimePickerTooltip = ({ timeRange, timeZone }: { timeRange: TimeRange; timeZone?: TimeZone }) => { const styles = useStyles2(getLabelStyles); diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index aa73bf3cd22..c525ce2e768 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -13411,7 +13411,8 @@ "forwards-time-aria-label": "Move time range forwards", "to": "to", "zoom-out-button": "Zoom out time range", - "zoom-out-tooltip": "Time range zoom out
CTRL+Z" + "zoom-out-tooltip": "Time range zoom out
CTRL+Z", + "zoom-out-tooltip-new": "Time range zoom out
t -" }, "time-range": { "apply": "Apply time range", From 399370bc2c7607a60074b898bd01865f2064f735 Mon Sep 17 00:00:00 2001 From: Jesse David Peterson Date: Wed, 26 Nov 2025 16:41:02 -0400 Subject: [PATCH 06/14] TimeRange: Avoid x-axis pan jump caused by data loading latency (#114496) * fix(time-range): avoid x-axis pan jump caused by data loading latency * refactor(time-range): use a more semantically meaningful names --- .../uPlot/config/UPlotConfigBuilder.ts | 2 +- .../XAxisInteractionAreaPlugin.test.tsx | 16 ++++++++++++++- .../plugins/XAxisInteractionAreaPlugin.tsx | 7 +++++-- .../app/core/components/TimeSeries/utils.ts | 20 +++++++++++++++++-- .../core/components/TimelineChart/utils.ts | 18 +++++++++++++++++ 5 files changed, 57 insertions(+), 6 deletions(-) diff --git a/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts b/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts index 27322714477..6bc91793236 100644 --- a/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts +++ b/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts @@ -29,7 +29,7 @@ const cursorDefaults: Cursor = { type PrepData = (frames: DataFrame[]) => AlignedData | FacetedData; type PreDataStacked = (frames: DataFrame[], stackingGroups: StackingGroup[]) => AlignedData | FacetedData; -type PlotState = { isPanning: false } | { isPanning: true; min: number; max: number }; +type PlotState = { isPanning: false } | { isPanning: true; min: number; max: number; isTimeRangePending?: boolean }; export class UPlotConfigBuilder { readonly uid = Math.random().toString(36).slice(2); diff --git a/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.test.tsx b/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.test.tsx index d9a6e787251..e9e471b82af 100644 --- a/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.test.tsx +++ b/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.test.tsx @@ -137,7 +137,7 @@ describe('XAxisInteractionAreaPlugin', () => { expect(mockQueryZoom).not.toHaveBeenCalled(); }); - it('should set isPanning state during drag and clear on mouseup', () => { + it('should set isPanning state during drag and mark isTimeRangePending on mouseup', () => { setupXAxisPan(asUPlot(mockUPlot), asConfigBuilder(mockConfigBuilder), mockQueryZoom); xAxisElement.dispatchEvent(new MouseEvent('mousedown', { clientX: 400, bubbles: true })); @@ -153,6 +153,20 @@ describe('XAxisInteractionAreaPlugin', () => { document.dispatchEvent(new MouseEvent('mouseup', { clientX: 350, bubbles: true })); + expect(mockConfigBuilder.setState).toHaveBeenCalledWith({ + isPanning: true, + min: expectedRange.from, + max: expectedRange.to, + isTimeRangePending: true, + }); + }); + + it('should clear isPanning state immediately for small drags below threshold', () => { + setupXAxisPan(asUPlot(mockUPlot), asConfigBuilder(mockConfigBuilder), mockQueryZoom); + + xAxisElement.dispatchEvent(new MouseEvent('mousedown', { clientX: 400, bubbles: true })); + document.dispatchEvent(new MouseEvent('mouseup', { clientX: 402, bubbles: true })); + expect(mockConfigBuilder.setState).toHaveBeenCalledWith({ isPanning: false }); }); }); diff --git a/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.tsx b/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.tsx index 694f372307d..5b70546b55a 100644 --- a/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.tsx +++ b/packages/grafana-ui/src/components/uPlot/plugins/XAxisInteractionAreaPlugin.tsx @@ -96,11 +96,14 @@ export const setupXAxisPan = ( xAxisEl.style.cursor = 'grab'; - config.setState({ isPanning: false }); + const isSignificantDrag = Math.abs(dragPixels) >= MIN_PAN_DIST; - if (Math.abs(dragPixels) >= MIN_PAN_DIST) { + if (isSignificantDrag) { const newRange = calculatePanRange(startMin, startMax, dragPixels, u.bbox.width); + config.setState({ isPanning: true, min: newRange.from, max: newRange.to, isTimeRangePending: true }); queryZoom(newRange); + } else { + config.setState({ isPanning: false }); } document.removeEventListener('mousemove', onMove); diff --git a/public/app/core/components/TimeSeries/utils.ts b/public/app/core/components/TimeSeries/utils.ts index f81de163b7f..88a82ec299c 100644 --- a/public/app/core/components/TimeSeries/utils.ts +++ b/public/app/core/components/TimeSeries/utils.ts @@ -138,10 +138,26 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ({ range: () => { const state = builder.getState(); if (state.isPanning) { + if (state.isTimeRangePending) { + const timeRange = getTimeRange(); + const propsFrom = timeRange.from.valueOf(); + const propsTo = timeRange.to.valueOf(); + + const MIN_TIMESPAN_MS = 1; + const fromMatches = Math.abs(propsFrom - state.min) <= MIN_TIMESPAN_MS; + const toMatches = Math.abs(propsTo - state.max) <= MIN_TIMESPAN_MS; + const timeRangeHasUpdated = fromMatches && toMatches; + + if (timeRangeHasUpdated) { + builder.setState({ isPanning: false }); + return [propsFrom, propsTo]; + } + } + return [state.min, state.max]; } - const r = getTimeRange(); - return [r.from.valueOf(), r.to.valueOf()]; + const timeRange = getTimeRange(); + return [timeRange.from.valueOf(), timeRange.to.valueOf()]; }, }); diff --git a/public/app/core/components/TimelineChart/utils.ts b/public/app/core/components/TimelineChart/utils.ts index 955f1ea11b2..264c5498be9 100644 --- a/public/app/core/components/TimelineChart/utils.ts +++ b/public/app/core/components/TimelineChart/utils.ts @@ -159,6 +159,24 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ( range: (u) => { const state = builder.getState(); if (state.isPanning) { + if (state.isTimeRangePending) { + const propsRange = coreConfig.xRange(u); + const propsFrom = propsRange[0]; + const propsTo = propsRange[1]; + + if (propsFrom != null && propsTo != null) { + const MIN_TIMESPAN_MS = 1; + const fromMatches = Math.abs(propsFrom - state.min) <= MIN_TIMESPAN_MS; + const toMatches = Math.abs(propsTo - state.max) <= MIN_TIMESPAN_MS; + const timeRangeHasUpdated = fromMatches && toMatches; + + if (timeRangeHasUpdated) { + builder.setState({ isPanning: false }); + return propsRange; + } + } + } + return [state.min, state.max]; } return coreConfig.xRange(u); From a455f9700d6b780133635f7d8af520669b2b0983 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Wed, 26 Nov 2025 22:07:24 +0100 Subject: [PATCH 07/14] Azure: Enable resource picker updates (#114089) * Default resource picker updates to true * Update toggle docs --- .../configure-grafana/feature-toggles/index.md | 2 +- packages/grafana-data/src/types/featureToggles.gen.ts | 2 +- pkg/services/featuremgmt/registry.go | 4 ++-- pkg/services/featuremgmt/toggles_gen.csv | 2 +- pkg/services/featuremgmt/toggles_gen.json | 11 +++++++---- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 7ca66464680..f0f12cce1b6 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -66,6 +66,7 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `grafanaAssistantInProfilesDrilldown` | Enables integration with Grafana Assistant in Profiles Drilldown | Yes | | `sharingDashboardImage` | Enables image sharing functionality for dashboards | Yes | | `tabularNumbers` | Use fixed-width numbers globally in the UI | | +| `azureResourcePickerUpdates` | Enables the updated Azure Monitor resource picker | Yes | | `tempoSearchBackendMigration` | Run search queries through the tempo backend | | ## Public preview feature toggles @@ -95,7 +96,6 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `localeFormatPreference` | Specifies the locale so the correct format for numbers and dates can be shown | | `logsPanelControls` | Enables a control component for the logs panel in Explore | | `interactiveLearning` | Enables the interactive learning app | -| `azureResourcePickerUpdates` | Enables the updated Azure Monitor resource picker | | `newVizSuggestions` | Enable new visualization suggestions | | `preventPanelChromeOverflow` | Restrict PanelChrome contents with overflow: hidden; | | `newPanelPadding` | Increases panel padding globally | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 40f2abe4d73..33750126afa 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -1091,7 +1091,7 @@ export interface FeatureToggles { graphiteBackendMode?: boolean; /** * Enables the updated Azure Monitor resource picker - * @default false + * @default true */ azureResourcePickerUpdates?: boolean; /** diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 91b59bf711d..37ee3001e99 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1801,10 +1801,10 @@ var ( { Name: "azureResourcePickerUpdates", Description: "Enables the updated Azure Monitor resource picker", - Stage: FeatureStagePublicPreview, + Stage: FeatureStageGeneralAvailability, FrontendOnly: true, Owner: grafanaPartnerPluginsSquad, - Expression: "false", + Expression: "true", }, { Name: "prometheusTypeMigration", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index f161087ecb1..f66bc50bbc7 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -245,7 +245,7 @@ teamFolders,experimental,@grafana/grafana-search-navigate-organise,false,false,f interactiveLearning,preview,@grafana/pathfinder,false,false,false alertingTriage,experimental,@grafana/alerting-squad,false,false,false graphiteBackendMode,privatePreview,@grafana/partner-datasources,false,false,false -azureResourcePickerUpdates,preview,@grafana/partner-datasources,false,false,true +azureResourcePickerUpdates,GA,@grafana/partner-datasources,false,false,true prometheusTypeMigration,experimental,@grafana/partner-datasources,false,true,false pluginContainers,privatePreview,@grafana/plugins-platform-backend,false,true,false tempoSearchBackendMigration,GA,@grafana/oss-big-tent,false,true,false diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 8dee7f8206b..46e5eda90a7 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -766,16 +766,19 @@ { "metadata": { "name": "azureResourcePickerUpdates", - "resourceVersion": "1763734583253", + "resourceVersion": "1764153435365", "creationTimestamp": "2025-07-31T22:56:50Z", - "deletionTimestamp": "2025-08-01T11:30:17Z" + "deletionTimestamp": "2025-08-01T11:30:17Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-11-26 10:37:15.365919 +0000 UTC" + } }, "spec": { "description": "Enables the updated Azure Monitor resource picker", - "stage": "preview", + "stage": "GA", "codeowner": "@grafana/partner-datasources", "frontend": true, - "expression": "false" + "expression": "true" } }, { From f116539541d7f9cd56140f546becc5a150ef4bde Mon Sep 17 00:00:00 2001 From: owensmallwood Date: Wed, 26 Nov 2025 15:53:00 -0600 Subject: [PATCH 08/14] Unified Storage: Update readme (#114415) * update readme * Adds message about creating database * update sample storage config --- pkg/storage/unified/README.md | 52 +++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/pkg/storage/unified/README.md b/pkg/storage/unified/README.md index 034147d84d9..7131178e8a1 100644 --- a/pkg/storage/unified/README.md +++ b/pkg/storage/unified/README.md @@ -202,30 +202,58 @@ then run: kubectl --kubeconfig=./grafana.kubeconfig create -f folder-generate.yaml ``` -### Run as a GRPC service +### Run as a separate GRPC service -#### Start GRPC storage-server +It is recommended to use a separate config file for the storage-server. Create a file `conf/storage-server.ini` with the following content: -Make sure you have the gRPC address in the `[grafana-apiserver]` section of your config file: ```ini +app_mode = development + +target = storage-server + +[database] +type = mysql +host = 127.0.0.1:3306 +name = unified-storage +user = root +password = rootpass +skip_migrations = true +ensure_default_org_and_user = false + +[grpc_server] +network = "tcp" +address = "127.0.0.1:10000" + [grafana-apiserver] -; your gRPC server address -address = localhost:10000 -``` +storage_type = unified -You also need the `[grpc_server_authentication]` section to authenticate incoming requests: -```ini [grpc_server_authentication] -; http url to Grafana's signing keys to validate incoming id tokens -signing_keys_url = http://localhost:3000/api/signing-keys/keys +signing_keys_url = http://localhost:3011/api/signing-keys/keys mode = "on-prem" + +[feature_toggles] +kubernetesDashboards = true +kubernetesFolders = true +unifiedStorage = true +unifiedStorageHistoryPruner = true +unifiedStorageSearch = true +unifiedStorageSearchPermissionFiltering = false +unifiedStorageSearchSprinkles = false + +[unified_storage] +enable_search = true +https_skip_verify = true ``` -This currently only works with a separate database configuration (see previous section). +You should also have a MySQL database running. You can create one with our docker blocks by running: +```bash +make devenv sources=mysql +``` +The database credentials in the example above will work with the default mysql docker block. You'll also need to create a database named `unified-storage`. Start the storage-server with: ```sh -GF_DEFAULT_TARGET=storage-server ./bin/grafana server target +./bin/grafana server target --config conf/storage-server.ini ``` The GRPC service will listen on port 10000 From a8aef11926176e442e6fa1cefbdf4b1ada06c828 Mon Sep 17 00:00:00 2001 From: Drew Slobodnjak <60050885+drew08t@users.noreply.github.com> Date: Wed, 26 Nov 2025 14:59:21 -0800 Subject: [PATCH 09/14] Geomap: Fix data filter for layers (#114515) * Geomap: Fix data filter for layers * Simplify comments --- .../plugins/panel/geomap/utils/layers.test.ts | 123 ++++++++++++++++++ .../app/plugins/panel/geomap/utils/layers.ts | 8 +- 2 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 public/app/plugins/panel/geomap/utils/layers.test.ts diff --git a/public/app/plugins/panel/geomap/utils/layers.test.ts b/public/app/plugins/panel/geomap/utils/layers.test.ts new file mode 100644 index 00000000000..1310d892481 --- /dev/null +++ b/public/app/plugins/panel/geomap/utils/layers.test.ts @@ -0,0 +1,123 @@ +jest.mock('ol-mapbox-style', () => ({})); +jest.mock('geotiff', () => ({})); + +import BaseLayer from 'ol/layer/Base'; + +import { + DataFrame, + DataQueryRequest, + FieldType, + LoadingState, + MapLayerHandler, + MapLayerOptions, + PanelData, + TimeRange, +} from '@grafana/data'; + +import { applyLayerFilter } from './layers'; + +describe('applyLayerFilter', () => { + const createDataFrame = (refId: string): DataFrame => ({ + refId, + fields: [{ name: 'value', type: FieldType.number, values: [1, 2, 3], config: {} }], + length: 3, + }); + + it('should apply filter when query exists and is visible', () => { + const update = jest.fn(); + const handler: MapLayerHandler = { + init: () => ({}) as BaseLayer, + update, + }; + const options: MapLayerOptions = { + name: 'Test', + type: 'markers', + filterData: { id: 'byRefId', options: 'A' }, + }; + const panelData: PanelData = { + series: [createDataFrame('A'), createDataFrame('B')], + state: LoadingState.Done, + timeRange: {} as TimeRange, + request: { targets: [{ refId: 'A' }, { refId: 'B' }] } as DataQueryRequest, + }; + + applyLayerFilter(handler, options, panelData); + + expect(update).toHaveBeenCalledWith( + expect.objectContaining({ + series: [createDataFrame('A')], + }) + ); + }); + + it('should return empty series when query exists but is hidden', () => { + const update = jest.fn(); + const handler: MapLayerHandler = { + init: () => ({}) as BaseLayer, + update, + }; + const options: MapLayerOptions = { + name: 'Test', + type: 'markers', + filterData: { id: 'byRefId', options: 'A' }, + }; + const panelData: PanelData = { + series: [createDataFrame('B')], + state: LoadingState.Done, + timeRange: {} as TimeRange, + request: { targets: [{ refId: 'A', hide: true }, { refId: 'B' }] } as DataQueryRequest, + }; + + applyLayerFilter(handler, options, panelData); + + expect(update).toHaveBeenCalledWith( + expect.objectContaining({ + series: [], + }) + ); + }); + + it('should not apply filter when query does not exist', () => { + const update = jest.fn(); + const handler: MapLayerHandler = { + init: () => ({}) as BaseLayer, + update, + }; + const options: MapLayerOptions = { + name: 'Test', + type: 'markers', + filterData: { id: 'byRefId', options: 'C' }, + }; + const panelData: PanelData = { + series: [createDataFrame('A'), createDataFrame('B')], + state: LoadingState.Done, + timeRange: {} as TimeRange, + request: { targets: [{ refId: 'A' }, { refId: 'B' }] } as DataQueryRequest, + }; + + applyLayerFilter(handler, options, panelData); + + expect(update).toHaveBeenCalledWith(panelData); + }); + + it('should pass through all data when no filter is configured', () => { + const update = jest.fn(); + const handler: MapLayerHandler = { + init: () => ({}) as BaseLayer, + update, + }; + const options: MapLayerOptions = { + name: 'Test', + type: 'markers', + }; + const panelData: PanelData = { + series: [createDataFrame('A'), createDataFrame('B')], + state: LoadingState.Done, + timeRange: {} as TimeRange, + }; + + applyLayerFilter(handler, options, panelData); + + expect(update).toHaveBeenCalledWith(panelData); + }); +}); diff --git a/public/app/plugins/panel/geomap/utils/layers.ts b/public/app/plugins/panel/geomap/utils/layers.ts index f20d8891ade..925ec1a82c4 100644 --- a/public/app/plugins/panel/geomap/utils/layers.ts +++ b/public/app/plugins/panel/geomap/utils/layers.ts @@ -26,7 +26,13 @@ export const applyLayerFilter = ( let panelData = panelDataProps; if (options.filterData) { const matcherFunc = getFrameMatchers(options.filterData); - if (panelData.series.some(matcherFunc)) { + + const queryExists = panelData.request?.targets.some((target) => { + return target.refId === options.filterData?.options; + }); + + // Only apply filter if the target query exists + if (queryExists) { panelData = { ...panelData, series: panelData.series.filter(matcherFunc), From 84a07be6e4e1acd8f064c3b390c30188d5703afc Mon Sep 17 00:00:00 2001 From: Eric Shields Date: Wed, 26 Nov 2025 15:47:32 -0800 Subject: [PATCH 10/14] Chore: Finalize removal of updateNode & expandOrFilter (#114202) - Remove references to, and related private functions for, `updateNode` and `expandOrFilter` - Remove obsolete tests - Update all usages of `updateNode` to `filterNode` - Integrate `expandOrFilter` functionality into `filterNode` - Add profiler to `filterNode` - Add `.claude` to `.gitignore` IDE junk section - Unit tests for `toggleExpandedNode` and `filterNode` - Add profiler to `toggleExpandedNode` Fixes: https://github.com/grafana/grafana-operator-experience-squad/issues/1566 --- .gitignore | 1 + .../actions/scopeActions.test.tsx | 28 +- .../commandPalette/actions/scopeActions.tsx | 10 +- .../commandPalette/actions/scopesUtils.ts | 5 +- .../selector/ScopesSelectorService.test.ts | 536 ++++++++++++------ .../scopes/selector/ScopesSelectorService.ts | 134 ++--- 6 files changed, 425 insertions(+), 289 deletions(-) diff --git a/.gitignore b/.gitignore index f1a90d05693..5302a698c2f 100644 --- a/.gitignore +++ b/.gitignore @@ -71,6 +71,7 @@ public/css/*.min.css .vs/ .cursor/ .devcontainer/ +.claude/ .eslintcache .stylelintcache diff --git a/public/app/features/commandPalette/actions/scopeActions.test.tsx b/public/app/features/commandPalette/actions/scopeActions.test.tsx index 0d828d66a81..126877224de 100644 --- a/public/app/features/commandPalette/actions/scopeActions.test.tsx +++ b/public/app/features/commandPalette/actions/scopeActions.test.tsx @@ -22,7 +22,7 @@ jest.mock('./scopesUtils', () => { }); const mockScopeServicesState = { - updateNode: jest.fn(), + filterNode: jest.fn(), selectScope: jest.fn(), resetSelection: jest.fn(), nodes: {}, @@ -99,12 +99,12 @@ describe('useRegisterScopesActions', () => { }); it('should register scope tree actions and return scopesRow when scopes are selected', () => { - const mockUpdateNode = jest.fn(); + const mockFilterNode = jest.fn(); // First run with empty scopes in the scopes service (useScopeServicesState as jest.Mock).mockReturnValue({ ...mockScopeServicesState, - updateNode: mockUpdateNode, + filterNode: mockFilterNode, selectedScopes: [{ scopeId: 'scope1', name: 'Scope 1' }], }); @@ -112,14 +112,14 @@ describe('useRegisterScopesActions', () => { return useRegisterScopesActions('', jest.fn()); }); - expect(mockUpdateNode).toHaveBeenCalledWith('', true, ''); + expect(mockFilterNode).toHaveBeenCalledWith('', ''); expect(useRegisterActions).toHaveBeenLastCalledWith([rootScopeAction], [[rootScopeAction]]); expect(result.current.scopesRow).toBeDefined(); // Simulate loading of scopes in the service (useScopeServicesState as jest.Mock).mockReturnValue({ ...mockScopeServicesState, - updateNode: mockUpdateNode, + filterNode: mockFilterNode, selectedScopes: [{ scopeId: 'scope1', name: 'Scope 1' }], nodes, tree, @@ -151,12 +151,12 @@ describe('useRegisterScopesActions', () => { }); it('should load next level of scopes', () => { - const mockUpdateNode = jest.fn(); + const mockFilterNode = jest.fn(); // First run with empty scopes in the scopes service (useScopeServicesState as jest.Mock).mockReturnValue({ ...mockScopeServicesState, - updateNode: mockUpdateNode, + filterNode: mockFilterNode, nodes, tree, }); @@ -165,7 +165,7 @@ describe('useRegisterScopesActions', () => { return useRegisterScopesActions('', jest.fn(), 'scopes/scope1'); }); - expect(mockUpdateNode).toHaveBeenCalledWith('scope1', true, ''); + expect(mockFilterNode).toHaveBeenCalledWith('scope1', ''); }); it('does not return component if no scopes are selected', () => { @@ -259,12 +259,12 @@ describe('useRegisterScopesActions', () => { }); it('should not use global scope search when searching in some deeper scope category', async () => { - const mockUpdateNode = jest.fn(); + const mockFilterNode = jest.fn(); // First run with empty scopes in the scopes service (useScopeServicesState as jest.Mock).mockReturnValue({ ...mockScopeServicesState, - updateNode: mockUpdateNode, + filterNode: mockFilterNode, nodes, tree, }); @@ -273,17 +273,17 @@ describe('useRegisterScopesActions', () => { return useRegisterScopesActions('something', jest.fn(), 'scopes/scope1'); }); - expect(mockUpdateNode).toHaveBeenCalledWith('scope1', true, 'something'); + expect(mockFilterNode).toHaveBeenCalledWith('scope1', 'something'); expect(mockScopeServicesState.searchAllNodes).not.toHaveBeenCalled(); }); it('should not use global scope search if feature flag is off', async () => { config.featureToggles.scopeSearchAllLevels = false; - const mockUpdateNode = jest.fn(); + const mockFilterNode = jest.fn(); // First run with empty scopes in the scopes service (useScopeServicesState as jest.Mock).mockReturnValue({ ...mockScopeServicesState, - updateNode: mockUpdateNode, + filterNode: mockFilterNode, nodes, tree, }); @@ -292,7 +292,7 @@ describe('useRegisterScopesActions', () => { return useRegisterScopesActions('something', jest.fn(), ''); }); - expect(mockUpdateNode).toHaveBeenCalledWith('', true, 'something'); + expect(mockFilterNode).toHaveBeenCalledWith('', 'something'); expect(mockScopeServicesState.searchAllNodes).not.toHaveBeenCalled(); }); diff --git a/public/app/features/commandPalette/actions/scopeActions.tsx b/public/app/features/commandPalette/actions/scopeActions.tsx index bde41152237..e85d136d2f6 100644 --- a/public/app/features/commandPalette/actions/scopeActions.tsx +++ b/public/app/features/commandPalette/actions/scopeActions.tsx @@ -58,22 +58,22 @@ export function useRegisterScopesActions( * @param parentId */ function useScopeTreeActions(searchQuery: string, parentId?: string | null) { - const { updateNode, selectScope, resetSelection, nodes, tree, selectedScopes } = useScopeServicesState(); + const { filterNode, selectScope, resetSelection, nodes, tree, selectedScopes } = useScopeServicesState(); // Initialize the scopes the first time this runs and reset the scopes that were selected on unmount. useEffect(() => { - updateNode('', true, ''); + filterNode('', ''); resetSelection(); return () => { resetSelection(); }; - }, [updateNode, resetSelection]); + }, [filterNode, resetSelection]); // Load the next level of scopes when the parentId changes. useEffect(() => { const parentScopeId = !parentId || parentId === 'scopes' ? '' : last(parentId.split('/'))!; - updateNode(parentScopeId, true, searchQuery); - }, [updateNode, searchQuery, parentId]); + filterNode(parentScopeId, searchQuery); + }, [filterNode, searchQuery, parentId]); return useMemo( () => mapScopesNodesTreeToActions(nodes, tree!, selectedScopes, selectScope), diff --git a/public/app/features/commandPalette/actions/scopesUtils.ts b/public/app/features/commandPalette/actions/scopesUtils.ts index 2848569247a..d269a58abb1 100644 --- a/public/app/features/commandPalette/actions/scopesUtils.ts +++ b/public/app/features/commandPalette/actions/scopesUtils.ts @@ -14,7 +14,7 @@ export function useScopeServicesState() { const services = useScopesServices(); if (!services) { return { - updateNode: () => {}, + filterNode: () => Promise.resolve(), selectScope: () => {}, resetSelection: () => {}, searchAllNodes: () => Promise.resolve([]), @@ -32,7 +32,7 @@ export function useScopeServicesState() { }, }; } - const { updateNode, filterNode, selectScope, resetSelection, searchAllNodes, deselectScope, apply, getScopeNodes } = + const { filterNode, selectScope, resetSelection, searchAllNodes, deselectScope, apply, getScopeNodes } = services.scopesSelectorService; const selectorServiceState: ScopesSelectorServiceState | undefined = useObservable( services.scopesSelectorService.stateObservable ?? new Observable(), @@ -42,7 +42,6 @@ export function useScopeServicesState() { return { getScopeNodes, filterNode, - updateNode, selectScope, resetSelection, searchAllNodes, diff --git a/public/app/features/scopes/selector/ScopesSelectorService.test.ts b/public/app/features/scopes/selector/ScopesSelectorService.test.ts index 57ff735f019..2c89ea58fe5 100644 --- a/public/app/features/scopes/selector/ScopesSelectorService.test.ts +++ b/public/app/features/scopes/selector/ScopesSelectorService.test.ts @@ -107,162 +107,9 @@ describe('ScopesSelectorService', () => { service = new ScopesSelectorService(apiClient, dashboardsService, store); }); - describe('updateNode', () => { - it('should update node and fetch children when expanded', async () => { - await service.updateNode('', true, ''); - expect(service.state.nodes['test-scope-node']).toEqual(mockNode); - expect(service.state.tree).toMatchObject({ - children: { 'test-scope-node': { expanded: false, scopeNodeId: 'test-scope-node' } }, - expanded: true, - query: '', - scopeNodeId: '', - }); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: '' }); - }); - - it.skip('should update node query and fetch children when query changes', async () => { - await service.updateNode('', true, ''); // Expand first - // Simulate a change in the query - await service.updateNode('', true, 'new-qu'); - await service.updateNode('', true, 'new-query'); - expect(service.state.tree).toMatchObject({ - children: {}, - expanded: true, - query: 'new-query', - scopeNodeId: '', - }); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: 'new-query' }); - }); - - it('should not fetch children when node is collapsed and query is unchanged', async () => { - // First expand the node - await service.updateNode('', true, ''); - // Then collapse it - await service.updateNode('', false, ''); - // Only the first expansion should trigger fetchNodes - expect(apiClient.fetchNodes).toHaveBeenCalledTimes(1); - }); - - it.skip('should clear query on first expansion but keep it when filtering within populated node', async () => { - const mockChildNode: ScopeNode = { - metadata: { name: 'child-node' }, - spec: { linkId: 'child-scope', linkType: 'scope', parentName: '', nodeType: 'leaf', title: 'child-node' }, - }; - - apiClient.fetchNodes.mockResolvedValue([mockChildNode]); - - // Scenario 1: First expansion (no children yet) - clear query for unfiltered view - await service.updateNode('', true, 'search-query'); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: undefined }); - - // Parent query should be cleared and child nodes should have no query (first expansion) - expect(service.state.tree?.query).toBe(''); - let childTreeNode = service.state.tree?.children?.['child-node']; - expect(childTreeNode?.query).toBe(''); - - // Scenario 2: Filtering within node that already has children - await service.updateNode('', true, 'new-search'); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: 'new-search' }); - - // Parent and child nodes should have the filter query (filtering within existing children) - expect(service.state.tree?.query).toBe('new-search'); - childTreeNode = service.state.tree?.children?.['child-node']; - expect(childTreeNode?.query).toBe('new-search'); - - expect(apiClient.fetchNodes).toHaveBeenCalledTimes(2); - }); - - it.skip('should always reset query on any expansion', async () => { - const mockChildNode: ScopeNode = { - metadata: { name: 'child-node' }, - spec: { linkId: 'child-scope', linkType: 'scope', parentName: '', nodeType: 'leaf', title: 'child-node' }, - }; - - apiClient.fetchNodes.mockResolvedValue([mockChildNode]); - - // First expansion with any query should reset parent query and not pass query to API - await service.updateNode('', true, 'some-search-query'); - - // Verify query is reset and API called without query for first expansion - expect(service.state.tree?.query).toBe(''); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: undefined }); - expect(service.state.tree?.children?.['child-node']?.query).toBe(''); - }); - - it.skip('should handle query reset correctly for nested levels beyond root', async () => { - // Set up mock nodes for multi-level hierarchy - const mockParentNode: ScopeNode = { - metadata: { name: 'parent-container' }, - spec: { linkId: '', linkType: 'scope', parentName: '', nodeType: 'container', title: 'Parent Container' }, - }; - - const mockChildNode: ScopeNode = { - metadata: { name: 'child-container' }, - spec: { - linkId: '', - linkType: 'scope', - parentName: 'parent-container', - nodeType: 'container', - title: 'Child Container', - }, - }; - - const mockGrandchildNode: ScopeNode = { - metadata: { name: 'grandchild-leaf' }, - spec: { - linkId: 'leaf-scope', - linkType: 'scope', - parentName: 'child-container', - nodeType: 'leaf', - title: 'Grandchild Leaf', - }, - }; - - // Mock different responses for different parent nodes - apiClient.fetchNodes.mockImplementation((options: { parent?: string; query?: string; limit?: number }) => { - if (options.parent === '') { - return Promise.resolve([mockParentNode]); - } else if (options.parent === 'parent-container') { - return Promise.resolve([mockChildNode]); - } else if (options.parent === 'child-container') { - return Promise.resolve([mockGrandchildNode]); - } - return Promise.resolve([]); - }); - - // Step 1: Expand root node with search query - await service.updateNode('', true, 'search-query'); - - // Root should have query reset, API called without query - expect(service.state.tree?.query).toBe(''); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: undefined }); - expect(service.state.tree?.children?.['parent-container']?.query).toBe(''); - - // Step 2: Expand first-level child with search query - await service.updateNode('parent-container', true, 'open-search-query'); - - // First-level child should have query reset, API called without query - const parentContainer = service.state.tree?.children?.['parent-container']; - expect(parentContainer?.query).toBe(''); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: 'parent-container', query: undefined }); - expect(parentContainer?.children?.['child-container']?.query).toBe(''); - - // Step 3: Now filter within the first-level child (second call to same node) - await service.updateNode('parent-container', true, 'filter-search'); - - // Now both parent and children should show the filter query since we're filtering within existing children - const newParentContainer = service.state.tree?.children?.['parent-container']; - expect(newParentContainer?.query).toBe('filter-search'); - expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: 'parent-container', query: 'filter-search' }); - expect(newParentContainer?.children?.['child-container']?.query).toBe('filter-search'); - - expect(apiClient.fetchNodes).toHaveBeenCalledTimes(3); - }); - }); - describe('selectScope and deselectScope', () => { beforeEach(async () => { - await service.updateNode('', true, ''); + await service.filterNode('', ''); }); it('should select a scope', async () => { @@ -311,7 +158,7 @@ describe('ScopesSelectorService', () => { it('should set parent node for recent scopes', async () => { // Load mock node - await service.updateNode('', true, ''); + await service.filterNode('', ''); await service.changeScopes(['test-scope'], 'test-scope-node'); expect(service.state.appliedScopes).toEqual([{ scopeId: 'test-scope', parentNodeId: 'test-scope-node' }]); @@ -363,7 +210,7 @@ describe('ScopesSelectorService', () => { describe('closeAndApply', () => { it('should close the selector and apply the selected scopes', async () => { - await service.updateNode('', true, ''); + await service.filterNode('', ''); await service.selectScope('test-scope-node'); await service.closeAndApply(); expect(service.state.opened).toBe(false); @@ -373,6 +220,7 @@ describe('ScopesSelectorService', () => { describe('apply', () => { it('should apply the selected scopes without closing the selector', async () => { + await service.filterNode('', ''); await service.open(); await service.selectScope('test-scope-node'); await service.apply(); @@ -391,7 +239,7 @@ describe('ScopesSelectorService', () => { describe('removeAllScopes', () => { it('should remove all selected and applied scopes', async () => { - await service.updateNode('', true, ''); + await service.filterNode('', ''); await service.selectScope('test-scope-node'); await service.apply(); await service.removeAllScopes(); @@ -399,7 +247,7 @@ describe('ScopesSelectorService', () => { }); it('should clear navigation scope when removing all scopes', async () => { - await service.updateNode('', true, ''); + await service.filterNode('', ''); await service.selectScope('test-scope-node'); await service.apply(); await service.removeAllScopes(); @@ -429,7 +277,7 @@ describe('ScopesSelectorService', () => { describe('getRecentScopes', () => { it('should parse and filter scopes', async () => { - await service.updateNode('', true, ''); + await service.filterNode('', ''); await service.selectScope('test-scope-node'); await service.apply(); storeValue[RECENT_SCOPES_KEY] = JSON.stringify([[mockScope2], [mockScope]]); @@ -439,7 +287,7 @@ describe('ScopesSelectorService', () => { }); it('should work with old version', async () => { - await service.updateNode('', true, ''); + await service.filterNode('', ''); await service.selectScope('test-scope-node'); await service.apply(); storeValue[RECENT_SCOPES_KEY] = JSON.stringify([ @@ -615,9 +463,347 @@ describe('ScopesSelectorService', () => { }); }); + describe('toggleExpandedNode', () => { + const expandableNode: ScopeNode = { + metadata: { name: 'expandable-node' }, + spec: { + linkId: '', + linkType: undefined, + parentName: '', + nodeType: 'container', + title: 'Expandable Node', + }, + }; + + const childNode: ScopeNode = { + metadata: { name: 'child-node' }, + spec: { + linkId: 'child-scope', + linkType: 'scope', + parentName: 'expandable-node', + nodeType: 'leaf', + title: 'Child Node', + }, + }; + + const leafNode: ScopeNode = { + metadata: { name: 'leaf-node' }, + spec: { + linkId: 'leaf-scope', + linkType: 'scope', + parentName: '', + nodeType: 'leaf', + title: 'Leaf Node', + }, + }; + + beforeEach(async () => { + // Mock fetchNodes to return different nodes based on parent + apiClient.fetchNodes = jest + .fn() + .mockImplementation((options: { parent?: string; query?: string; limit?: number }) => { + if (options.parent === '') { + return [expandableNode, leafNode]; + } else if (options.parent === 'expandable-node') { + return [childNode]; + } + return []; + }); + + // Load root nodes + await service.filterNode('', ''); + }); + + it('should expand a collapsed node and load its children', async () => { + // Node should start collapsed + expect(service.state.tree?.children?.['expandable-node']?.expanded).toBe(false); + + // Expand the node + await service.toggleExpandedNode('expandable-node'); + + // Node should now be expanded + expect(service.state.tree?.children?.['expandable-node']?.expanded).toBe(true); + // Children should be loaded + expect(service.state.tree?.children?.['expandable-node']?.children).toBeDefined(); + expect(service.state.tree?.children?.['expandable-node']?.children?.['child-node']).toBeDefined(); + }); + + it('should collapse an expanded node', async () => { + // First expand the node + await service.toggleExpandedNode('expandable-node'); + expect(service.state.tree?.children?.['expandable-node']?.expanded).toBe(true); + + // Now collapse it + await service.toggleExpandedNode('expandable-node'); + expect(service.state.tree?.children?.['expandable-node']?.expanded).toBe(false); + }); + + it('should reset query to empty string when toggling', async () => { + // First filter with a query + await service.filterNode('expandable-node', 'test-query'); + expect(service.state.tree?.children?.['expandable-node']?.query).toBe('test-query'); + + // Toggle the node + await service.toggleExpandedNode('expandable-node'); + + // Query should be reset + expect(service.state.tree?.children?.['expandable-node']?.query).toBe(''); + }); + + it('should throw error when node not found in tree', async () => { + await expect(service.toggleExpandedNode('non-existent-node')).rejects.toThrow( + 'Node non-existent-node not found in tree' + ); + }); + + it('should throw error when trying to toggle a non-expandable node', async () => { + await expect(service.toggleExpandedNode('leaf-node')).rejects.toThrow( + 'Trying to expand node at id leaf-node that is not expandable' + ); + }); + + it('should reload parent children when collapsing', async () => { + const fetchNodesSpy = jest.spyOn(apiClient, 'fetchNodes'); + + // Expand then collapse + await service.toggleExpandedNode('expandable-node'); + fetchNodesSpy.mockClear(); + + await service.toggleExpandedNode('expandable-node'); + + // Should reload parent's (root) children + expect(fetchNodesSpy).toHaveBeenCalledWith({ parent: '', query: '' }); + }); + + it('should reload parent children with parent query when collapsing', async () => { + // First filter the root with a query + await service.filterNode('', 'parent-query'); + + // Expand a node + await service.toggleExpandedNode('expandable-node'); + + const fetchNodesSpy = jest.spyOn(apiClient, 'fetchNodes'); + + // Collapse the node + await service.toggleExpandedNode('expandable-node'); + + // Should reload parent's children with parent's query + expect(fetchNodesSpy).toHaveBeenCalledWith({ parent: '', query: 'parent-query' }); + }); + }); + + describe('filterNode', () => { + const containerNode: ScopeNode = { + metadata: { name: 'container-node' }, + spec: { + linkId: '', + linkType: undefined, + parentName: '', + nodeType: 'container', + title: 'Container Node', + }, + }; + + const filteredChild: ScopeNode = { + metadata: { name: 'filtered-child' }, + spec: { + linkId: 'filtered-scope', + linkType: 'scope', + parentName: 'container-node', + nodeType: 'leaf', + title: 'Filtered Child', + }, + }; + + const leafNode: ScopeNode = { + metadata: { name: 'leaf-node-2' }, + spec: { + linkId: 'leaf-scope-2', + linkType: 'scope', + parentName: '', + nodeType: 'leaf', + title: 'Leaf Node 2', + }, + }; + + beforeEach(async () => { + // Mock fetchNodes to return different nodes based on query + apiClient.fetchNodes = jest + .fn() + .mockImplementation((options: { parent?: string; query?: string; limit?: number }) => { + if (options.parent === '' && !options.query) { + return [containerNode, leafNode]; + } else if (options.parent === 'container-node' && options.query === 'test-filter') { + return [filteredChild]; + } else if (options.parent === 'container-node' && !options.query) { + return [filteredChild]; + } + return []; + }); + + // Load root nodes + await service.filterNode('', ''); + }); + + it('should filter node with non-empty query', async () => { + await service.filterNode('container-node', 'test-filter'); + + // Node should be expanded + expect(service.state.tree?.children?.['container-node']?.expanded).toBe(true); + // Query should be set + expect(service.state.tree?.children?.['container-node']?.query).toBe('test-filter'); + }); + + it('should load children with the query parameter', async () => { + const fetchNodesSpy = jest.spyOn(apiClient, 'fetchNodes'); + + await service.filterNode('container-node', 'my-query'); + + expect(fetchNodesSpy).toHaveBeenCalledWith({ parent: 'container-node', query: 'my-query' }); + }); + + it('should set expanded to true when filtering', async () => { + // Node starts collapsed + expect(service.state.tree?.children?.['container-node']?.expanded).toBe(false); + + await service.filterNode('container-node', 'test-filter'); + + // Should be expanded after filtering + expect(service.state.tree?.children?.['container-node']?.expanded).toBe(true); + }); + + it('should throw error when node not found', async () => { + await expect(service.filterNode('non-existent-node', 'query')).rejects.toThrow( + 'Trying to filter node at path or id non-existent-node not found' + ); + }); + + it('should throw error when trying to filter a non-expandable node', async () => { + await expect(service.filterNode('leaf-node-2', 'query')).rejects.toThrow( + 'Trying to filter node at id leaf-node-2 that is not expandable' + ); + }); + + it('should handle multiple calls with different queries', async () => { + // First filter + await service.filterNode('container-node', 'first-query'); + expect(service.state.tree?.children?.['container-node']?.query).toBe('first-query'); + + // Second filter with different query + await service.filterNode('container-node', 'second-query'); + expect(service.state.tree?.children?.['container-node']?.query).toBe('second-query'); + + // Third filter with empty query + await service.filterNode('container-node', ''); + expect(service.state.tree?.children?.['container-node']?.query).toBe(''); + }); + + it('should start profiler interaction', async () => { + const profiler = { + startInteraction: jest.fn(), + stopInteraction: jest.fn(), + }; + + // Create new service with profiler + const serviceWithProfiler = new ScopesSelectorService(apiClient, dashboardsService, store, profiler as never); + + await serviceWithProfiler.filterNode('', ''); + + expect(profiler.startInteraction).toHaveBeenCalledWith('scopeNodeFilter'); + expect(profiler.stopInteraction).toHaveBeenCalled(); + }); + + it('should stop profiler even when error is thrown', async () => { + const profiler = { + startInteraction: jest.fn(), + stopInteraction: jest.fn(), + }; + + const serviceWithProfiler = new ScopesSelectorService(apiClient, dashboardsService, store, profiler as never); + + // Load initial nodes + await serviceWithProfiler.filterNode('', ''); + + // Try to filter a non-existent node + await expect(serviceWithProfiler.filterNode('non-existent', 'query')).rejects.toThrow(); + + // Profiler should still be stopped + expect(profiler.stopInteraction).toHaveBeenCalled(); + }); + }); + + describe('interaction between toggleExpandedNode and filterNode', () => { + const expandableNode: ScopeNode = { + metadata: { name: 'interaction-node' }, + spec: { + linkId: '', + linkType: undefined, + parentName: '', + nodeType: 'container', + title: 'Interaction Node', + }, + }; + + const childNode: ScopeNode = { + metadata: { name: 'interaction-child' }, + spec: { + linkId: 'child-scope', + linkType: 'scope', + parentName: 'interaction-node', + nodeType: 'leaf', + title: 'Child Node', + }, + }; + + beforeEach(async () => { + apiClient.fetchNodes = jest + .fn() + .mockImplementation((options: { parent?: string; query?: string; limit?: number }) => { + if (options.parent === '') { + return [expandableNode]; + } else if (options.parent === 'interaction-node') { + return [childNode]; + } + return []; + }); + + await service.filterNode('', ''); + }); + + it('should clear query when toggleExpandedNode is called after filterNode', async () => { + // Filter with a query + await service.filterNode('interaction-node', 'test-query'); + expect(service.state.tree?.children?.['interaction-node']?.query).toBe('test-query'); + + // Toggle should clear the query + await service.toggleExpandedNode('interaction-node'); + expect(service.state.tree?.children?.['interaction-node']?.query).toBe(''); + }); + + it('should set query when filterNode is called after toggleExpandedNode', async () => { + // First toggle (expand) + await service.toggleExpandedNode('interaction-node'); + expect(service.state.tree?.children?.['interaction-node']?.query).toBe(''); + + // Filter should set the query + await service.filterNode('interaction-node', 'new-query'); + expect(service.state.tree?.children?.['interaction-node']?.query).toBe('new-query'); + }); + + it('should maintain expanded state when filtering an already expanded node', async () => { + // Expand the node + await service.toggleExpandedNode('interaction-node'); + expect(service.state.tree?.children?.['interaction-node']?.expanded).toBe(true); + + // Filter should keep it expanded + await service.filterNode('interaction-node', 'query'); + expect(service.state.tree?.children?.['interaction-node']?.expanded).toBe(true); + }); + }); + describe('redirect on scope selection', () => { it('should redirect to the first scopeNavigation with /d/ URL when current URL is not a scopeNavigation', async () => { - const mockNavigations: ScopeNavigation[] = [ + dashboardsService.state.scopeNavigations = [ { spec: { scope: 'test-scope', @@ -632,8 +818,6 @@ describe('ScopesSelectorService', () => { }, }, ]; - - dashboardsService.state.scopeNavigations = mockNavigations; (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' }); await service.changeScopes(['test-scope']); @@ -642,7 +826,7 @@ describe('ScopesSelectorService', () => { }); it('should NOT redirect when the first scopeNavigation does not contain /d/ (e.g., logs drilldown)', async () => { - const mockNavigations: ScopeNavigation[] = [ + dashboardsService.state.scopeNavigations = [ { spec: { scope: 'test-scope', @@ -657,8 +841,6 @@ describe('ScopesSelectorService', () => { }, }, ]; - - dashboardsService.state.scopeNavigations = mockNavigations; (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' }); await service.changeScopes(['test-scope']); @@ -667,7 +849,7 @@ describe('ScopesSelectorService', () => { }); it('should NOT redirect when current URL matches a scopeNavigation', async () => { - const mockNavigations: ScopeNavigation[] = [ + dashboardsService.state.scopeNavigations = [ { spec: { scope: 'test-scope', @@ -682,8 +864,6 @@ describe('ScopesSelectorService', () => { }, }, ]; - - dashboardsService.state.scopeNavigations = mockNavigations; (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/d/dashboard1' }); await service.changeScopes(['test-scope']); @@ -701,7 +881,7 @@ describe('ScopesSelectorService', () => { }); it('should NOT redirect when scopeNavigation does not have a url property', async () => { - const mockNavigations = [ + dashboardsService.state.scopeNavigations = [ { spec: { scope: 'test-scope', @@ -716,8 +896,6 @@ describe('ScopesSelectorService', () => { }, }, ] as unknown as ScopeNavigation[]; - - dashboardsService.state.scopeNavigations = mockNavigations; (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' }); await service.changeScopes(['test-scope']); @@ -726,7 +904,7 @@ describe('ScopesSelectorService', () => { }); it('should handle multiple scopeNavigations and redirect to the first dashboard one', async () => { - const mockNavigations: ScopeNavigation[] = [ + dashboardsService.state.scopeNavigations = [ { spec: { scope: 'test-scope', @@ -754,8 +932,6 @@ describe('ScopesSelectorService', () => { }, }, ]; - - dashboardsService.state.scopeNavigations = mockNavigations; (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' }); await service.changeScopes(['test-scope']); @@ -790,7 +966,7 @@ describe('ScopesSelectorService', () => { }); // First update the node to populate the service state - await service.updateNode('', true, ''); + await service.filterNode('', ''); // Then select the scope to set scopeNodeId in selectedScopes await service.selectScope('test-scope-node'); @@ -837,7 +1013,7 @@ describe('ScopesSelectorService', () => { (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' }); // First update the node to populate the service state - await service.updateNode('', true, ''); + await service.filterNode('', ''); // Then select the scope to set scopeNodeId in selectedScopes await service.selectScope('test-scope-node'); @@ -851,16 +1027,14 @@ describe('ScopesSelectorService', () => { }); it('should fall back to scope navigation when scope node is undefined', async () => { - const mockNavigations: ScopeNavigation[] = [ + // Don't add the node to the service state, so it will be undefined + dashboardsService.state.scopeNavigations = [ { spec: { scope: 'test-scope', url: '/d/dashboard1' }, status: { title: 'Dashboard 1', groups: [] }, metadata: { name: 'dashboard1' }, }, ]; - - // Don't add the node to the service state, so it will be undefined - dashboardsService.state.scopeNavigations = mockNavigations; (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' }); await service.changeScopes(['test-scope']); diff --git a/public/app/features/scopes/selector/ScopesSelectorService.ts b/public/app/features/scopes/selector/ScopesSelectorService.ts index 14a16be69ac..9dea729ff88 100644 --- a/public/app/features/scopes/selector/ScopesSelectorService.ts +++ b/public/app/features/scopes/selector/ScopesSelectorService.ts @@ -129,81 +129,38 @@ export class ScopesSelectorService extends ScopesServiceBase { - const path = getPathOfNode(scopeNodeId, this.state.nodes); - const nodeToToggle = treeNodeAtPath(this.state.tree!, path); - - if (!nodeToToggle) { - throw new Error(`Node ${scopeNodeId} not found in tree`); - } - - if (nodeToToggle.scopeNodeId !== '' && !isNodeExpandable(this.state.nodes[nodeToToggle.scopeNodeId])) { - throw new Error(`Trying to expand node at id ${scopeNodeId} that is not expandable`); - } - - const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => { - treeNode.expanded = !nodeToToggle.expanded; - treeNode.query = ''; - }); - - this.updateState({ tree: newTree }); - // If we are collapsing, we need to make sure that all the parent's children are avilable - if (nodeToToggle.expanded === true) { - const parentPath = path.slice(0, -1); - const parentNode = treeNodeAtPath(this.state.tree!, parentPath); - if (parentNode) { - await this.loadNodeChildren(parentPath, parentNode, parentNode.query); - } - } else { - await this.loadNodeChildren(path, nodeToToggle); - } - }; - - public filterNode = async (scopeNodeId: string, query: string) => { - const path = getPathOfNode(scopeNodeId, this.state.nodes); - const nodeToFilter = treeNodeAtPath(this.state.tree!, path); - - if (!nodeToFilter) { - throw new Error(`Trying to filter node at path or id ${scopeNodeId} not found`); - } - - if (nodeToFilter.scopeNodeId !== '' && !isNodeExpandable(this.state.nodes[nodeToFilter.scopeNodeId])) { - throw new Error(`Trying to filter node at id ${scopeNodeId} that is not expandable`); - } - - const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => { - treeNode.expanded = true; - treeNode.query = query; - }); - this.updateState({ tree: newTree }); - - await this.loadNodeChildren(path, nodeToFilter, query); - }; - - private expandOrFilterNode = async (scopeNodeId: string, query?: string) => { - this.interactionProfiler?.startInteraction('scopeNodeDiscovery'); - - const path = getPathOfNode(scopeNodeId, this.state.nodes); - - const nodeToExpand = treeNodeAtPath(this.state.tree!, path); + this.interactionProfiler?.startInteraction('scopeToggleExpandedNode'); try { - if (!nodeToExpand) { + const path = getPathOfNode(scopeNodeId, this.state.nodes); + const nodeToToggle = treeNodeAtPath(this.state.tree!, path); + + if (!nodeToToggle) { throw new Error(`Node ${scopeNodeId} not found in tree`); } - if (nodeToExpand.scopeNodeId !== '' && !isNodeExpandable(this.state.nodes[nodeToExpand.scopeNodeId])) { + if (nodeToToggle.scopeNodeId !== '' && !isNodeExpandable(this.state.nodes[nodeToToggle.scopeNodeId])) { throw new Error(`Trying to expand node at id ${scopeNodeId} that is not expandable`); } - if (!nodeToExpand.expanded || nodeToExpand.query !== query) { - const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => { - treeNode.expanded = true; - treeNode.query = query || ''; - }); - this.updateState({ tree: newTree }); + const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => { + treeNode.expanded = !nodeToToggle.expanded; + treeNode.query = ''; + }); - await this.loadNodeChildren(path, nodeToExpand, query); + this.updateState({ tree: newTree }); + // If we are collapsing, we need to make sure that all the parent's children are available + if (nodeToToggle.expanded) { + const parentPath = path.slice(0, -1); + const parentNode = treeNodeAtPath(this.state.tree!, parentPath); + if (parentNode) { + await this.loadNodeChildren(parentPath, parentNode, parentNode.query); + } + } else { + await this.loadNodeChildren(path, nodeToToggle); } + // Catch and throw error so we can ensure the profiler is stopped + // todo: leverage component-level } catch (error) { throw error; } finally { @@ -211,20 +168,34 @@ export class ScopesSelectorService extends ScopesServiceBase { - const path = getPathOfNode(scopeNodeId, this.state.nodes); + public filterNode = async (scopeNodeId: string, query: string) => { + this.interactionProfiler?.startInteraction('scopeNodeFilter'); - const nodeToCollapse = treeNodeAtPath(this.state.tree!, path); + try { + const path = getPathOfNode(scopeNodeId, this.state.nodes); + const nodeToFilter = treeNodeAtPath(this.state.tree!, path); - if (!nodeToCollapse) { - throw new Error(`Trying to collapse node at path or id ${scopeNodeId} not found`); + if (!nodeToFilter) { + throw new Error(`Trying to filter node at path or id ${scopeNodeId} not found`); + } + + if (nodeToFilter.scopeNodeId !== '' && !isNodeExpandable(this.state.nodes[nodeToFilter.scopeNodeId])) { + throw new Error(`Trying to filter node at id ${scopeNodeId} that is not expandable`); + } + + const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => { + treeNode.expanded = true; + treeNode.query = query; + }); + this.updateState({ tree: newTree }); + + await this.loadNodeChildren(path, nodeToFilter, query); + // Catch and throw error so we can ensure the profiler is stopped + } catch (error) { + throw error; + } finally { + this.interactionProfiler?.stopInteraction(); } - - const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => { - treeNode.expanded = false; - treeNode.query = ''; - }); - this.updateState({ tree: newTree }); }; private loadNodeChildren = async (path: string[], treeNode: TreeNode, query?: string) => { @@ -332,15 +303,6 @@ export class ScopesSelectorService extends ScopesServiceBase { - if (expanded) { - return this.expandOrFilterNode(scopeNodeId, query); - } - return this.collapseNode(scopeNodeId); - }; - changeScopes = (scopeNames: string[], parentNodeId?: string, scopeNodeId?: string, redirectOnApply?: boolean) => { return this.applyScopes( scopeNames.map((id, index) => ({ @@ -494,7 +456,7 @@ export class ScopesSelectorService extends ScopesServiceBase { if (!this.state.tree?.children || Object.keys(this.state.tree?.children).length === 0) { - await this.expandOrFilterNode(''); + await this.filterNode('', ''); } // First close all nodes From cb05a4ae1bb9f0875b75b48c530767db1031ebae Mon Sep 17 00:00:00 2001 From: Yunwen Zheng Date: Thu, 27 Nov 2025 01:03:36 -0500 Subject: [PATCH 11/14] usePullRequestParam: Provisioned dashboard preview banner, decode pull request url before sanitize (#114516) usePullRequestParam: decode url before sanitize --- .../components/Shared/PreviewBannerViewPR.tsx | 11 ++++++++--- .../provisioning/hooks/usePullRequestParam.ts | 8 ++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/public/app/features/provisioning/components/Shared/PreviewBannerViewPR.tsx b/public/app/features/provisioning/components/Shared/PreviewBannerViewPR.tsx index 8a796ee049c..15b814a14db 100644 --- a/public/app/features/provisioning/components/Shared/PreviewBannerViewPR.tsx +++ b/public/app/features/provisioning/components/Shared/PreviewBannerViewPR.tsx @@ -107,9 +107,14 @@ export function PreviewBannerViewPR({ prParam, isNewPr, behindBranch, repoUrl, b {/* when repo type is not local, we show branch information */} {showBranchInfo(repoType, branchInfo) && ( - branch: - {targetBranch} {'\u2192'}{' '} - {configuredBranch} + branch:{' '} + + {targetBranch} + {' '} + {'\u2192'}{' '} + + {configuredBranch} + )} diff --git a/public/app/features/provisioning/hooks/usePullRequestParam.ts b/public/app/features/provisioning/hooks/usePullRequestParam.ts index 81d10210503..8ad124953a4 100644 --- a/public/app/features/provisioning/hooks/usePullRequestParam.ts +++ b/public/app/features/provisioning/hooks/usePullRequestParam.ts @@ -9,9 +9,9 @@ export const usePullRequestParam = () => { const repoType = params.get('repo_type'); return { - prURL: prParam ? textUtil.sanitizeUrl(prParam) : undefined, - newPrURL: newPrParam ? textUtil.sanitizeUrl(newPrParam) : undefined, - repoURL: repoUrl ? textUtil.sanitizeUrl(repoUrl) : undefined, - repoType: repoType ? textUtil.sanitizeUrl(repoType) : undefined, + prURL: prParam ? textUtil.sanitizeUrl(decodeURIComponent(prParam)) : undefined, + newPrURL: newPrParam ? textUtil.sanitizeUrl(decodeURIComponent(newPrParam)) : undefined, + repoURL: repoUrl ? textUtil.sanitizeUrl(decodeURIComponent(repoUrl)) : undefined, + repoType: repoType ? textUtil.sanitizeUrl(decodeURIComponent(repoType)) : undefined, }; }; From b47352478791e6b673d667bd2c0073a4e4b2a781 Mon Sep 17 00:00:00 2001 From: Yunwen Zheng Date: Thu, 27 Nov 2025 02:01:40 -0500 Subject: [PATCH 12/14] Provisioning: View in repository open containing folder (#114513) * Provisioning: View in repostiory open containing folder * i18n * comment * tweaks * Simplify for display --------- Co-authored-by: Clarity-89 --- .../Repository/RepositoryLink.tsx | 4 +- ...ositoryCard.tsx => RepositoryListItem.tsx} | 22 ++--- .../provisioning/Shared/RepositoryList.tsx | 4 +- public/app/features/provisioning/utils/git.ts | 98 +++++++++++++------ public/locales/en-US/grafana.json | 3 - 5 files changed, 79 insertions(+), 52 deletions(-) rename public/app/features/provisioning/Repository/{RepositoryCard.tsx => RepositoryListItem.tsx} (84%) diff --git a/public/app/features/provisioning/Repository/RepositoryLink.tsx b/public/app/features/provisioning/Repository/RepositoryLink.tsx index f97a448efa4..fe4dbd4b9c5 100644 --- a/public/app/features/provisioning/Repository/RepositoryLink.tsx +++ b/public/app/features/provisioning/Repository/RepositoryLink.tsx @@ -4,7 +4,7 @@ import { Trans } from '@grafana/i18n'; import { LinkButton, Stack, Text, TextLink } from '@grafana/ui'; import { useGetRepositoryQuery } from 'app/api/clients/provisioning/v0alpha1'; -import { getRepoHref } from '../utils/git'; +import { getRepoHrefForProvider } from '../utils/git'; type RepositoryLinkProps = { name?: string; @@ -19,7 +19,7 @@ export function RepositoryLink({ name, jobType }: RepositoryLinkProps) { return null; } - const repoHref = getRepoHref(repo.spec?.github); + const repoHref = getRepoHrefForProvider(repo.spec); if (jobType === 'sync') { return ( diff --git a/public/app/features/provisioning/Repository/RepositoryCard.tsx b/public/app/features/provisioning/Repository/RepositoryListItem.tsx similarity index 84% rename from public/app/features/provisioning/Repository/RepositoryCard.tsx rename to public/app/features/provisioning/Repository/RepositoryListItem.tsx index 8a8205e9a2f..841ec3bd928 100644 --- a/public/app/features/provisioning/Repository/RepositoryCard.tsx +++ b/public/app/features/provisioning/Repository/RepositoryListItem.tsx @@ -2,12 +2,13 @@ import { ReactNode } from 'react'; import { t, Trans } from '@grafana/i18n'; import { reportInteraction } from '@grafana/runtime'; -import { Stack, Text, TextLink, Icon, Card, LinkButton, Badge } from '@grafana/ui'; +import { Badge, Card, LinkButton, Stack, Text, TextLink } from '@grafana/ui'; import { Repository, ResourceCount } from 'app/api/clients/provisioning/v0alpha1'; import { RepoIcon } from '../Shared/RepoIcon'; import { StatusBadge } from '../Shared/StatusBadge'; import { PROVISIONING_URL } from '../constants'; +import { getRepoHrefForProvider } from '../utils/git'; import { getIsReadOnlyWorkflows } from '../utils/repository'; import { SyncRepository } from './SyncRepository'; @@ -16,7 +17,7 @@ interface Props { repository: Repository; } -export function RepositoryCard({ repository }: Props) { +export function RepositoryListItem({ repository }: Props) { const isReadOnlyRepo = getIsReadOnlyWorkflows(repository.spec?.workflows); const { metadata, spec, status } = repository; const name = metadata?.name ?? ''; @@ -27,24 +28,13 @@ export function RepositoryCard({ repository }: Props) { if (spec?.type === 'github') { const { url = '', branch } = spec.github ?? {}; const branchUrl = branch ? `${url}/tree/${branch}` : url; + const href = getRepoHrefForProvider(spec) || branchUrl; meta.push( - - {branchUrl} + + {href.split('/').slice(3).join('/')} ); - - if (status?.webhook?.id) { - const webhookUrl = `${url}/settings/hooks/${status.webhook.id}`; - meta.push( - - - Webhook - - - - ); - } } else if (spec?.type === 'local') { meta.push( diff --git a/public/app/features/provisioning/Shared/RepositoryList.tsx b/public/app/features/provisioning/Shared/RepositoryList.tsx index 3bfadfe872a..038359c6e8b 100644 --- a/public/app/features/provisioning/Shared/RepositoryList.tsx +++ b/public/app/features/provisioning/Shared/RepositoryList.tsx @@ -4,7 +4,7 @@ import { t, Trans } from '@grafana/i18n'; import { Alert, Box, EmptyState, FilterInput, Icon, Stack, TextLink } from '@grafana/ui'; import { Repository } from 'app/api/clients/provisioning/v0alpha1'; -import { RepositoryCard } from '../Repository/RepositoryCard'; +import { RepositoryListItem } from '../Repository/RepositoryListItem'; import { useResourceStats } from '../Wizard/hooks/useResourceStats'; import { UPGRADE_URL } from '../constants'; import { useIsProvisionedInstance } from '../hooks/useIsProvisionedInstance'; @@ -89,7 +89,7 @@ export function RepositoryList({ items }: Props) { )} {filteredItems.length ? ( - filteredItems.map((item) => ) + filteredItems.map((item) => ) ) : ( { return `${github.url}/tree/${github.branch}`; }; +// Remove leading and trailing slashes from a string. +const stripSlashes = (s: string) => s.replace(/^\/+|\/+$/g, ''); + +// Split a path into segments and URL-encode each segment. +// Ensures the final URL remains valid for all providers (GitHub, GitLab, etc.). +const splitAndEncode = (s: string) => stripSlashes(s).split('/').map(encodeURIComponent); + +type BuildRepoUrlParams = { + baseUrl?: string; + branch?: string | null; + providerSegments: string[]; + path?: string | null; +}; + +const buildRepoUrl = ({ baseUrl, branch, providerSegments, path }: BuildRepoUrlParams) => { + if (!baseUrl) { + return undefined; + } + + // Normalize base URL: trim whitespace + remove trailing slashes. + const cleanBase = stripSlashes(baseUrl.trim()); + const cleanBranch = branch?.trim() || undefined; + + // Start composing URL parts: + // base URL + provider-specific segments (e.g., "tree", "blob", etc.) + const parts = [cleanBase, ...providerSegments]; + + // Append the branch name if present. + if (cleanBranch) { + parts.push(cleanBranch); + } + + // Append encoded path segments if provided. + // This ensures nested files like "src/utils/index.ts" produce safe URLs. + if (path) { + parts.push(...splitAndEncode(path.trim())); + } + + return parts.join('/'); +}; + export const getRepoHrefForProvider = (spec?: RepositorySpec) => { if (!spec || !spec.type) { return undefined; } switch (spec.type) { - case 'github': { - const url = spec.github?.url; - const branch = spec.github?.branch; - if (!url) { - return undefined; - } - return branch ? `${url}/tree/${branch}` : url; - } - - case 'gitlab': { - const url = spec.gitlab?.url; - const branch = spec.gitlab?.branch; - if (!url) { - return undefined; - } - return branch ? `${url}/-/tree/${branch}` : url; - } - case 'bitbucket': { - const url = spec.bitbucket?.url; - const branch = spec.bitbucket?.branch; - if (!url) { - return undefined; - } - return branch ? `${url}/src/${branch}` : url; - } - case 'git': { - // Return a generic URL for pure git repositories - return spec.git?.url; - } + case 'github': + return buildRepoUrl({ + baseUrl: spec.github?.url, + branch: spec.github?.branch, + providerSegments: ['tree'], + path: spec.github?.path, + }); + case 'gitlab': + return buildRepoUrl({ + baseUrl: spec.gitlab?.url, + branch: spec.gitlab?.branch, + providerSegments: ['-', 'tree'], + path: spec.gitlab?.path, + }); + case 'bitbucket': + return buildRepoUrl({ + baseUrl: spec.bitbucket?.url, + branch: spec.bitbucket?.branch, + providerSegments: ['src'], + path: spec.bitbucket?.path, + }); + case 'git': + return buildRepoUrl({ + baseUrl: spec.git?.url, + branch: spec.git?.branch, + providerSegments: ['tree'], + path: spec.git?.path, + }); default: return undefined; } diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index c525ce2e768..5a55ab0843f 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -11920,9 +11920,6 @@ "source-code": "Source code" }, "repository-card": { - "get-repository-meta": { - "webhook": "Webhook" - }, "read-only-badge": "Read only", "settings": "Settings", "view": "View" From b90b8f2a3410e8b602a04c265226b9498a289fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 27 Nov 2025 11:00:43 +0100 Subject: [PATCH 13/14] Dashboard: Sidebar / outline style fixes (#114487) * Dashboard: Sidebar / outline style fixes * fixing empty outline node * fixing empty outline node --- eslint-suppressions.json | 5 --- .../edit-pane/DashboardOutline.tsx | 39 ++++++++++++++----- .../scene/DashboardControls.tsx | 2 +- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index fde5e53587d..3f5773bf84f 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1912,11 +1912,6 @@ "count": 4 } }, - "public/app/features/dashboard-scene/edit-pane/DashboardOutline.tsx": { - "@typescript-eslint/consistent-type-assertions": { - "count": 1 - } - }, "public/app/features/dashboard-scene/inspect/HelpWizard/HelpWizard.tsx": { "no-restricted-syntax": { "count": 3 diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardOutline.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardOutline.tsx index 4ce6536ac5c..97e17c34780 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardOutline.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardOutline.tsx @@ -5,7 +5,7 @@ import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { Trans, t } from '@grafana/i18n'; import { SceneObject } from '@grafana/scenes'; -import { Box, Icon, Sidebar, Stack, Text, useElementSelection, useStyles2 } from '@grafana/ui'; +import { Box, Icon, Sidebar, Text, useElementSelection, useStyles2 } from '@grafana/ui'; import { isRepeatCloneOrChildOf } from '../utils/clone'; import { DashboardInteractions } from '../utils/interactions'; @@ -85,6 +85,7 @@ function DashboardOutlineNode({ sceneObject, editPane, isEditing, depth, index } aria-selected={isSelected} className={styles.container} onClick={onNodeClicked} + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions style={{ '--depth': depth } as React.CSSProperties} >
@@ -99,7 +100,7 @@ function DashboardOutlineNode({ sceneObject, editPane, isEditing, depth, index } )}