From e1a2f178e7459986d8c10bb04d5fffc67d5652fc Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Wed, 26 Nov 2025 13:41:06 +0100 Subject: [PATCH 01/11] App Plugins: Allow to define experimental pages (#114232) --- pkg/middleware/auth.go | 29 ++++++ pkg/middleware/auth_test.go | 96 ++++++++++++++++++++ pkg/services/navtree/navtreeimpl/applinks.go | 5 + 3 files changed, 130 insertions(+) diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index 719d2ab5cb5..f013d9d2bfa 100644 --- a/pkg/middleware/auth.go +++ b/pkg/middleware/auth.go @@ -1,6 +1,7 @@ package middleware import ( + "context" "errors" "net/http" "net/url" @@ -21,6 +22,13 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" + "github.com/open-feature/go-sdk/openfeature" +) + +var openfeatureClient = openfeature.NewDefaultClient() + +const ( + pluginPageFeatureFlagPrefix = "plugin-page-visible." ) type AuthOptions struct { @@ -146,6 +154,12 @@ func RoleAppPluginAuth(accessControl ac.AccessControl, ps pluginstore.Store, log return } + if !PageIsFeatureToggleEnabled(c.Req.Context(), c.Req.URL.Path) { + logger.Debug("Forbidden experimental plugin page", "plugin", pluginID, "path", c.Req.URL.Path) + accessForbidden(c) + return + } + permitted := true path := normalizeIncludePath(c.Req.URL.Path) hasAccess := ac.HasAccess(accessControl, c) @@ -294,3 +308,18 @@ func shouldForceLogin(c *contextmodel.ReqContext) bool { return forceLogin } + +// PageIsFeatureToggleEnabled checks if a page is enabled via OpenFeature feature flags. +// It returns false if the feature flag is set and set to false. +// The feature flag key format is: "plugin-page-visible." +func PageIsFeatureToggleEnabled(ctx context.Context, path string) bool { + flagKey := pluginPageFeatureFlagPrefix + filepath.Clean(path) + enabled := openfeatureClient.Boolean( + ctx, + flagKey, + true, + openfeature.TransactionContext(ctx), + ) + + return enabled +} diff --git a/pkg/middleware/auth_test.go b/pkg/middleware/auth_test.go index fdca1d04ee3..19a7d68559e 100644 --- a/pkg/middleware/auth_test.go +++ b/pkg/middleware/auth_test.go @@ -1,12 +1,17 @@ package middleware import ( + "context" "errors" "fmt" "net/http" "net/http/httptest" + "sync" "testing" + "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk/openfeature/memprovider" + oftesting "github.com/open-feature/go-sdk/openfeature/testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,6 +33,8 @@ import ( "github.com/grafana/grafana/pkg/web" ) +var openfeatureTestMutex sync.Mutex + func setupAuthMiddlewareTest(t *testing.T, identity *authn.Identity, authErr error) *contexthandler.ContextHandler { return contexthandler.ProvideService(setting.NewCfg(), &authntest.FakeService{ ExpectedErr: authErr, @@ -422,6 +429,60 @@ func TestCanAdminPlugin(t *testing.T) { } } +func TestPageIsFeatureToggleEnabled(t *testing.T) { + type testCase struct { + desc string + path string + flags map[string]bool + expectedResult bool + } + + tests := []testCase{ + { + desc: "returns true when feature flag is enabled", + path: "/a/my-plugin/settings", + flags: map[string]bool{ + pluginPageFeatureFlagPrefix + "/a/my-plugin/settings": true, + }, + expectedResult: true, + }, + { + desc: "returns false when feature flag is disabled", + path: "/a/my-plugin/settings", + flags: map[string]bool{ + pluginPageFeatureFlagPrefix + "/a/my-plugin/settings": false, + }, + expectedResult: false, + }, + { + desc: "returns false when feature flag is disabled with trailing slash", + path: "/a/my-plugin/settings/", + flags: map[string]bool{ + pluginPageFeatureFlagPrefix + "/a/my-plugin/settings": false, + }, + expectedResult: false, + }, + { + desc: "returns true when feature flag does not exist", + path: "/a/my-plugin/settings", + flags: map[string]bool{}, + expectedResult: true, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + ctx := context.Background() + + setupTestProvider(t, tt.flags) + + result := PageIsFeatureToggleEnabled(ctx, tt.path) + + assert.Equal(t, tt.expectedResult, result) + }) + } +} + func contextProvider(modifiers ...func(c *contextmodel.ReqContext)) web.Handler { return func(c *web.Context) { reqCtx := &contextmodel.ReqContext{ @@ -437,3 +498,38 @@ func contextProvider(modifiers ...func(c *contextmodel.ReqContext)) web.Handler c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx)) } } + +// setupTestProvider creates a test OpenFeature provider with the given flags. +// Uses a global lock to prevent concurrent provider changes across tests. +func setupTestProvider(t *testing.T, flags map[string]bool) oftesting.TestProvider { + t.Helper() + + // Lock to prevent concurrent provider changes + openfeatureTestMutex.Lock() + + testProvider := oftesting.NewTestProvider() + flagsMap := map[string]memprovider.InMemoryFlag{} + + for key, value := range flags { + flagsMap[key] = memprovider.InMemoryFlag{ + DefaultVariant: "defaultVariant", + Variants: map[string]any{ + "defaultVariant": value, + }, + } + } + + testProvider.UsingFlags(t, flagsMap) + + err := openfeature.SetProviderAndWait(testProvider) + require.NoError(t, err) + + t.Cleanup(func() { + testProvider.Cleanup() + _ = openfeature.SetProviderAndWait(openfeature.NoopProvider{}) + // Unlock after cleanup to allow other tests to run + openfeatureTestMutex.Unlock() + }) + + return testProvider +} diff --git a/pkg/services/navtree/navtreeimpl/applinks.go b/pkg/services/navtree/navtreeimpl/applinks.go index 4f8015f5687..b9b0edac970 100644 --- a/pkg/services/navtree/navtreeimpl/applinks.go +++ b/pkg/services/navtree/navtreeimpl/applinks.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/plugins" ac "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" @@ -128,6 +129,10 @@ func (s *ServiceImpl) processAppPlugin(plugin pluginstore.Plugin, c *contextmode } if include.Type == "page" { + if !middleware.PageIsFeatureToggleEnabled(c.Req.Context(), include.Path) { + s.log.Debug("Skipping page", "plugin", plugin.ID, "path", include.Path) + continue + } link := &navtree.NavLink{ Text: include.Name, Icon: include.Icon, From 5ba3139d4a055ca5f0c54a4e6f743412c92ca4e3 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 26 Nov 2025 13:51:16 +0100 Subject: [PATCH 02/11] E2E Selectors: Fix readme typo (#114480) fix typo --- packages/grafana-e2e-selectors/src/selectors/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grafana-e2e-selectors/src/selectors/README.md b/packages/grafana-e2e-selectors/src/selectors/README.md index ef419dac1fa..134197e6a6a 100644 --- a/packages/grafana-e2e-selectors/src/selectors/README.md +++ b/packages/grafana-e2e-selectors/src/selectors/README.md @@ -16,7 +16,7 @@ const components = { A few things to keep in mind: -- Strive to use e2e selector for all components in grafana/ui. +- Strive to use e2e selectors for all components in grafana/ui. - Don't ever delete selectors. Even though a selector may not be used in the Grafana repository, it can still be used in external plugins. - Only create new selector in case you're creating a new piece of UI. If you're changing an existing piece of UI that already has a selector defined, you need to keep using that selector. Otherwise you might break plugin end-to-end tests. - Prefer using string selectors in favour of function selectors. The purpose of the selectors is to provide a canonical way to select elements. From c94bf34d0bb2ff3be8beee61e5dc9f2e164a9bbb Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Wed, 26 Nov 2025 14:27:31 +0100 Subject: [PATCH 03/11] Alerting: Patch missing expression model refIds (#114477) --- .../components/expressions/Expression.tsx | 2 - .../rule-editor/ExpressionsEditor.tsx | 11 +-- .../QueryAndExpressionsStep.tsx | 4 -- .../__snapshots__/reducer.test.tsx.snap | 52 -------------- .../reducer.test.tsx | 17 ----- .../query-and-alert-condition/reducer.ts | 17 ----- .../alerting/unified/utils/rule-form.test.ts | 69 +++++++++++++++++++ .../alerting/unified/utils/rule-form.ts | 59 +++++++++++----- 8 files changed, 115 insertions(+), 116 deletions(-) diff --git a/public/app/features/alerting/unified/components/expressions/Expression.tsx b/public/app/features/alerting/unified/components/expressions/Expression.tsx index 79f805d52ad..0e5d5fd4b79 100644 --- a/public/app/features/alerting/unified/components/expressions/Expression.tsx +++ b/public/app/features/alerting/unified/components/expressions/Expression.tsx @@ -48,7 +48,6 @@ interface ExpressionProps { onSetCondition: (refId: string) => void; onUpdateRefId: (oldRefId: string, newRefId: string) => void; onRemoveExpression: (refId: string) => void; - onUpdateExpressionType: (refId: string, type: ExpressionQueryType) => void; onChangeQuery: (query: ExpressionQuery) => void; } @@ -62,7 +61,6 @@ export const Expression: FC = ({ onSetCondition, onUpdateRefId, onRemoveExpression, - onUpdateExpressionType, // this method is not used? maybe we should remove it onChangeQuery, }) => { const styles = useStyles2(getStyles); diff --git a/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx b/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx index 87952c4b19f..bf1a9985ec2 100644 --- a/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx @@ -4,7 +4,7 @@ import { useMemo } from 'react'; import { GrafanaTheme2, PanelData } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; import { isExpressionQuery } from 'app/features/expressions/guards'; -import { ExpressionQuery, ExpressionQueryType } from 'app/features/expressions/types'; +import { ExpressionQuery } from 'app/features/expressions/types'; import { AlertQuery } from 'app/types/unified-alerting-dto'; import { Expression } from '../expressions/Expression'; @@ -18,7 +18,6 @@ interface Props { queries: AlertQuery[]; onRemoveExpression: (refId: string) => void; onUpdateRefId: (oldRefId: string, newRefId: string) => void; - onUpdateExpressionType: (refId: string, type: ExpressionQueryType) => void; onUpdateQueryExpression: (query: ExpressionQuery) => void; } @@ -29,12 +28,15 @@ export const ExpressionsEditor = ({ panelData, onUpdateRefId, onRemoveExpression, - onUpdateExpressionType, onUpdateQueryExpression, }: Props) => { const expressionQueries = useMemo(() => { return queries.reduce((acc: ExpressionQuery[], query) => { - return isExpressionQuery(query.model) ? acc.concat(query.model) : acc; + if (isExpressionQuery(query.model)) { + acc.push(query.model); + } + + return acc; }, []); }, [queries]); const styles = useStyles2(getStyles); @@ -64,7 +66,6 @@ export const ExpressionsEditor = ({ onSetCondition={onSetCondition} onRemoveExpression={onRemoveExpression} onUpdateRefId={onUpdateRefId} - onUpdateExpressionType={onUpdateExpressionType} onChangeQuery={onUpdateQueryExpression} /> ); diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx index 844443a4dff..d19ac4bb619 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx @@ -73,7 +73,6 @@ import { updateExpression, updateExpressionRefId, updateExpressionTimeRange, - updateExpressionType, } from './reducer'; import { useAdvancedMode } from './useAdvancedMode'; import { useAlertQueryRunner } from './useAlertQueryRunner'; @@ -591,9 +590,6 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange, mod dispatch(removeExpression(refId)); }} onUpdateRefId={onUpdateRefId} - onUpdateExpressionType={(refId, type) => { - dispatch(updateExpressionType({ refId, type })); - }} onUpdateQueryExpression={(model) => { dispatch(updateExpression(model)); }} diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/reducer.test.tsx.snap b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/reducer.test.tsx.snap index 292a6904f80..33f3b0ea4e8 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/reducer.test.tsx.snap +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/reducer.test.tsx.snap @@ -442,55 +442,3 @@ exports[`Query and expressions reducer should update an expression refId and rew ], } `; - -exports[`Query and expressions reducer should update expression type 1`] = ` -{ - "queries": [ - { - "datasourceUid": "abc123", - "model": { - "refId": "A", - }, - "queryType": "query", - "refId": "A", - }, - { - "datasourceUid": "__expr__", - "model": { - "conditions": [ - { - "evaluator": { - "params": [ - 0, - 0, - ], - "type": "gt", - }, - "operator": { - "type": "and", - }, - "query": { - "params": [], - }, - "reducer": { - "params": [], - "type": "avg", - }, - "type": "query", - }, - ], - "datasource": { - "name": "Expression", - "type": "__expr__", - "uid": "__expr__", - }, - "expression": "", - "refId": "B", - "type": "reduce", - }, - "queryType": "", - "refId": "B", - }, - ], -} -`; diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx index 31858408509..a4a214b8348 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.test.tsx @@ -22,7 +22,6 @@ import { updateExpression, updateExpressionRefId, updateExpressionTimeRange, - updateExpressionType, } from './reducer'; const reduceExpression: AlertQuery = { @@ -388,22 +387,6 @@ describe('Query and expressions reducer', () => { expect(newState).toMatchSnapshot(); }); - - it('should update expression type', () => { - const initialState: QueriesAndExpressionsState = { - queries: [alertQuery, expressionQuery], - }; - - const newState = queriesAndExpressionsReducer( - initialState, - updateExpressionType({ - refId: 'B', - type: ExpressionQueryType.reduce, - }) - ); - - expect(newState).toMatchSnapshot(); - }); it('should remove first reducer', () => { const initialState: QueriesAndExpressionsState = { queries: [alertQuery, reduceExpression, thresholdExpression], diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts index 372e028bbcc..1514533cce0 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/reducer.ts @@ -283,23 +283,6 @@ export const queriesAndExpressionsReducer = createReducer(initialState, (builder queryType: 'expression', }); } - }) - .addCase(updateExpressionType, (state, action) => { - state.queries = state.queries.map((query) => { - return query.refId === action.payload.refId - ? { - ...query, - model: { - ...expressionDatasource.newQuery({ - type: action.payload.type, - conditions: [{ ...defaultCondition, query: { params: [] } }], - expression: '', - }), - refId: action.payload.refId, - }, - } - : query; - }); }); }); diff --git a/public/app/features/alerting/unified/utils/rule-form.test.ts b/public/app/features/alerting/unified/utils/rule-form.test.ts index 2e9b7a5730b..a5410e6ebd6 100644 --- a/public/app/features/alerting/unified/utils/rule-form.test.ts +++ b/public/app/features/alerting/unified/utils/rule-form.test.ts @@ -7,6 +7,7 @@ import { GrafanaAlertStateDecision, GrafanaRuleDefinition, RulerAlertingRuleDTO, + RulerGrafanaRuleDTO, } from 'app/types/unified-alerting-dto'; import { EvalFunction } from '../../state/alertDef'; @@ -20,6 +21,7 @@ import { alertingRulerRuleToRuleForm, cleanAnnotations, cleanLabels, + fixMissingRefIdsInExpressionModel, formValuesToRulerGrafanaRuleDTO, formValuesToRulerRuleDTO, getContactPointsFromDTO, @@ -520,3 +522,70 @@ describe('getDefaultExpressions', () => { expect(thresholdModel.expression).toBe('X'); }); }); + +describe('fixMissingRefIdsInExpressionModel', () => { + it('should return non-Grafana managed rules unchanged', () => { + const cloudAlertingRule: RulerAlertingRuleDTO = { + alert: 'CloudAlert', + expr: 'up == 0', + for: '5m', + labels: { severity: 'critical' }, + annotations: { summary: 'Instance down' }, + }; + + const result = fixMissingRefIdsInExpressionModel(cloudAlertingRule); + + expect(result).toEqual(cloudAlertingRule); + expect(result).toBe(cloudAlertingRule); // should be the exact same reference + }); + + it('should copy refId from query to model when model.refId is missing in Grafana managed rules', () => { + const ruleWithMissingRefId: RulerGrafanaRuleDTO = { + grafana_alert: { + uid: 'test-uid', + title: 'Test Alert', + namespace_uid: 'namespace-uid', + rule_group: 'test-group', + condition: 'B', + no_data_state: GrafanaAlertStateDecision.NoData, + exec_err_state: GrafanaAlertStateDecision.Alerting, + is_paused: false, + data: [ + { + refId: 'A', + datasourceUid: 'datasource-uid', + queryType: '', + relativeTimeRange: { from: 600, to: 0 }, + // @ts-ignore + model: { + // refId is missing here + datasource: { + type: 'grafana-testdata-datasource', + uid: 'PD8C576611E62080A', + }, + }, + }, + { + refId: 'B', + datasourceUid: ExpressionDatasourceUID, + queryType: '', + // @ts-ignore + model: { + // refId is missing here + type: ExpressionQueryType.reduce, + expression: 'A', + }, + }, + ], + }, + for: '5m', + labels: {}, + annotations: {}, + }; + + const result = fixMissingRefIdsInExpressionModel(ruleWithMissingRefId); + + expect(result.grafana_alert.data[0].model.refId).toBe('A'); + expect(result.grafana_alert.data[1].model.refId).toBe('B'); + }); +}); diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index 077934e21e1..0e83a276851 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -1,3 +1,5 @@ +import { produce } from 'immer'; + import { DataSourceInstanceSettings, IntervalValues, @@ -278,14 +280,16 @@ function getEditorSettingsFromDTO(ga: GrafanaRuleDefinition) { export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleFormValues { const { ruleSourceName, namespace, group, rule } = ruleWithLocation; - const isGrafanaRecordingRule = rulerRuleType.grafana.recordingRule(rule); + const normalizedRule = fixMissingRefIdsInExpressionModel(rule); + + const isGrafanaRecordingRule = rulerRuleType.grafana.recordingRule(normalizedRule); const defaultFormValues = getDefaultFormValues(isGrafanaRecordingRule ? RuleFormType.grafanaRecording : undefined); if (isGrafanaRulesSource(ruleSourceName)) { // GRAFANA-MANAGED RULES if (isGrafanaRecordingRule) { // grafana recording rule - const ga = rule.grafana_alert; + const ga = normalizedRule.grafana_alert; return { ...defaultFormValues, name: ga.title, @@ -294,16 +298,16 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF evaluateEvery: group.interval || defaultFormValues.evaluateEvery, queries: ga.data, condition: ga.condition, - annotations: normalizeDefaultAnnotations(listifyLabelsOrAnnotations(rule.annotations, false)), - labels: listifyLabelsOrAnnotations(rule.labels, true), + annotations: normalizeDefaultAnnotations(listifyLabelsOrAnnotations(normalizedRule.annotations, false)), + labels: listifyLabelsOrAnnotations(normalizedRule.labels, true), folder: { title: namespace, uid: ga.namespace_uid }, isPaused: ga.is_paused, metric: ga.record?.metric, targetDatasourceUid: ga.record?.target_datasource_uid || defaultFormValues.targetDatasourceUid, }; - } else if (rulerRuleType.grafana.rule(rule)) { + } else if (rulerRuleType.grafana.rule(normalizedRule)) { // grafana alerting rule - const ga = rule.grafana_alert; + const ga = normalizedRule.grafana_alert; const routingSettings: AlertManagerManualRouting | undefined = getContactPointsFromDTO(ga); if (ga.no_data_state !== undefined && ga.exec_err_state !== undefined) { return { @@ -312,14 +316,14 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF type: RuleFormType.grafana, group: group.name, evaluateEvery: group.interval || defaultFormValues.evaluateEvery, - evaluateFor: rule.for || '0', - keepFiringFor: rule.keep_firing_for || '0', + evaluateFor: normalizedRule.for || '0', + keepFiringFor: normalizedRule.keep_firing_for || '0', noDataState: ga.no_data_state, execErrState: ga.exec_err_state, queries: ga.data, condition: ga.condition, - annotations: normalizeDefaultAnnotations(listifyLabelsOrAnnotations(rule.annotations, false)), - labels: listifyLabelsOrAnnotations(rule.labels, true), + annotations: normalizeDefaultAnnotations(listifyLabelsOrAnnotations(normalizedRule.annotations, false)), + labels: listifyLabelsOrAnnotations(normalizedRule.labels, true), folder: { title: namespace, uid: ga.namespace_uid }, isPaused: ga.is_paused, @@ -338,7 +342,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF } } else { // DATASOURCE-MANAGED RULES - if (rulerRuleType.dataSource.alertingRule(rule)) { + if (rulerRuleType.dataSource.alertingRule(normalizedRule)) { const datasourceUid = getDataSourceSrv().getInstanceSettings(ruleSourceName)?.uid ?? ''; const defaultQuery = { @@ -346,27 +350,27 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF datasourceUid, queryType: '', relativeTimeRange: getDefaultRelativeTimeRange(), - expr: rule.expr, + expr: normalizedRule.expr, model: { refId: 'A', hide: false, - expr: rule.expr, + expr: normalizedRule.expr, }, }; - const alertingRuleValues = alertingRulerRuleToRuleForm(rule); + const alertingRuleValues = alertingRulerRuleToRuleForm(normalizedRule); return { ...defaultFormValues, ...alertingRuleValues, queries: [defaultQuery], - annotations: normalizeDefaultAnnotations(listifyLabelsOrAnnotations(rule.annotations, false)), + annotations: normalizeDefaultAnnotations(listifyLabelsOrAnnotations(normalizedRule.annotations, false)), type: RuleFormType.cloudAlerting, dataSourceName: ruleSourceName, namespace, group: group.name, }; - } else if (rulerRuleType.dataSource.recordingRule(rule)) { + } else if (rulerRuleType.dataSource.recordingRule(normalizedRule)) { const datasourceUid = getDataSourceSrv().getInstanceSettings(ruleSourceName)?.uid ?? ''; const defaultQuery = { @@ -374,15 +378,15 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF datasourceUid, queryType: '', relativeTimeRange: getDefaultRelativeTimeRange(), - expr: rule.expr, + expr: normalizedRule.expr, model: { refId: 'A', hide: false, - expr: rule.expr, + expr: normalizedRule.expr, }, }; - const recordingRuleValues = recordingRulerRuleToRuleForm(rule); + const recordingRuleValues = recordingRulerRuleToRuleForm(normalizedRule); return { ...defaultFormValues, @@ -399,6 +403,23 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF } } +/** + * This function isn't supposed to be needed, but we've noticed some customers are creating rules via Provisioning or + * other interfaces where they aren't including the RefId in the "model" of the expression so copy the refId from the query definition. + */ +export function fixMissingRefIdsInExpressionModel(rule: T): T { + // non-Grafana managed rules don't use expression nodes so we return the rule as-is + if (!rulerRuleType.grafana.rule(rule)) { + return rule; + } + + return produce(rule, (draft) => { + draft.grafana_alert.data.forEach((query) => { + query.model.refId = query.model.refId ?? query.refId; + }); + }); +} + export function grafanaRuleDtoToFormValues(rule: RulerGrafanaRuleDTO, namespace: string): RuleFormValues { const isGrafanaRecordingRule = rulerRuleType.grafana.recordingRule(rule); const defaultFormValues = getDefaultFormValues(isGrafanaRecordingRule ? RuleFormType.grafanaRecording : undefined); From f0a394e67b02eaade338bdf50ae7b272d63b01e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 26 Nov 2025 14:30:18 +0100 Subject: [PATCH 04/11] Dashboards: Use new sidebar in dynamic dashboards (#114245) * Dynamic dashboards sidebar wip * Progress * Outline in view mode cannot change name * Only one pane at a time * Adding starbutton and custom precence * undo / redo working * Progress * Update * Progress * Update * Public badge, provisining badge * playlist stop * Update * some initial unit tests * Fix export tooltip * Close pane when leaving edit mode if pane is an element * Update * e2e fixes * fixing e2e * Fix lint suppressions * e2e fixes * fixing more e2e * fixing e2e * fix e2e * e2e fixes * Fixinfg e2e * fixing e2e --- .../dashboard-group-panels.spec.ts | 24 +- .../dashboard-outline.spec.ts | 3 + .../dashboards-add-panel.spec.ts | 3 + .../dashboards-edit-custom-variables.spec.ts | 1 + .../dashboards-panel-layouts.spec.ts | 37 ++- .../dashboards-repeats-auto-grid.spec.ts | 36 +- .../dashboards-repeats-custom-grid.spec.ts | 4 +- .../dashboards-repeats-tabs-layout.spec.ts | 8 +- .../dashboards-title-description.spec.ts | 6 +- e2e-playwright/dashboard-new-layouts/utils.ts | 1 + eslint-suppressions.json | 5 - .../src/selectors/components.ts | 8 +- .../src/selectors/pages.ts | 8 + .../src/components/Sidebar/Sidebar.tsx | 3 +- .../src/components/Sidebar/SidebarButton.tsx | 60 ++-- .../components/Sidebar/SidebarPaneHeader.tsx | 2 + .../src/components/Sidebar/useSidebar.tsx | 6 +- .../edit-pane/DashboardEditPane.tsx | 51 ++- .../DashboardEditPaneRenderer.test.tsx | 69 ++-- .../edit-pane/DashboardEditPaneRenderer.tsx | 307 ++++++++---------- .../edit-pane/DashboardEditPaneSplitter.tsx | 178 +++++----- .../edit-pane/DashboardEditableElement.tsx | 30 +- .../edit-pane/DashboardExportButton.tsx | 57 ++++ .../edit-pane/DashboardOutline.test.tsx | 2 +- .../edit-pane/DashboardOutline.tsx | 25 +- .../edit-pane/EditPaneHeader.tsx | 38 +-- .../dashboard-scene/edit-pane/shared.ts | 5 - .../edit-pane/useEditableElement.ts | 21 -- .../edit-pane/useOutlineRename.tsx | 6 +- .../scene/DashboardControls.tsx | 42 ++- .../dashboard-scene/scene/DashboardScene.tsx | 10 +- .../scene/GoToSnapshotOriginButton.tsx | 2 +- .../scene/NavToolbarActions.tsx | 4 +- .../scene/new-toolbar/RightActions.tsx | 96 +----- .../actions/ExportDashboardButton.tsx | 39 --- .../actions/OpenSnapshotOriginButton.tsx | 2 +- .../new-toolbar/actions/SaveDashboard.tsx | 12 +- .../actions/ShareExportDashboardButton.tsx | 4 +- .../new-toolbar/actions/ToolbarSwitch.tsx | 14 +- .../variables/VariableEditableElement.tsx | 8 +- public/locales/en-US/grafana.json | 47 ++- 41 files changed, 634 insertions(+), 650 deletions(-) create mode 100644 public/app/features/dashboard-scene/edit-pane/DashboardExportButton.tsx delete mode 100644 public/app/features/dashboard-scene/edit-pane/useEditableElement.ts delete mode 100644 public/app/features/dashboard-scene/scene/new-toolbar/actions/ExportDashboardButton.tsx diff --git a/e2e-playwright/dashboard-new-layouts/dashboard-group-panels.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboard-group-panels.spec.ts index ffdd52e24f6..f077f48a7e0 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboard-group-panels.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboard-group-panels.spec.ts @@ -414,13 +414,13 @@ test.describe( ).toBeVisible(); // Go back to dashboard options - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click({ force: true }); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); // Expand layouts section await page.getByLabel('Expand Group layout category').click(); // Select tabs layout - await page.getByLabel('Tabs').click(); + await page.getByLabel('layout-selection-option-Tabs').click(); await expect(dashboardPage.getByGrafanaSelector(selectors.components.Tab.title('New row'))).toBeVisible(); await expect(dashboardPage.getByGrafanaSelector(selectors.components.Tab.title('New row 1'))).toBeVisible(); @@ -518,14 +518,14 @@ test.describe( await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.RowsLayout.titleInput) .fill('Test row 1'); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // clear the title input to simulate no title and click away to trigger onBlur await dashboardPage.getByGrafanaSelector(selectors.components.DashboardRow.title('Test row 1')).click(); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.RowsLayout.titleInput) .fill(''); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // title should be set to a default name await expect( @@ -543,14 +543,14 @@ test.describe( await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.RowsLayout.titleInput) .fill('Test row 2'); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // clear the title input to simulate no title and click away to trigger onBlur await dashboardPage.getByGrafanaSelector(selectors.components.DashboardRow.title('Test row 2')).click(); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.RowsLayout.titleInput) .fill(''); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // title should be set to a default name + 1 to avoid duplicates await expect( @@ -755,13 +755,13 @@ test.describe( await expect(dashboardPage.getByGrafanaSelector(selectors.components.Tab.title('New tab 2'))).toBeVisible(); // Go back to dashboard options - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click({ force: true }); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); // Expand layouts section await page.getByLabel('Expand Group layout category').click(); // Select rows layout - await page.getByLabel('Rows').click(); + await page.getByLabel('layout-selection-option-Rows').click(); await dashboardPage .getByGrafanaSelector(selectors.components.DashboardRow.wrapper('New tab 1')) @@ -903,14 +903,14 @@ test.describe( await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.TabsLayout.titleInput) .fill('Test tab 1'); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // clear the title input to simulate no title and click away to trigger onBlur await dashboardPage.getByGrafanaSelector(selectors.components.Tab.title('Test tab 1')).click(); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.TabsLayout.titleInput) .fill(''); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // title should be set to a default name await expect(dashboardPage.getByGrafanaSelector(selectors.components.Tab.title('New tab'))).toBeVisible(); @@ -923,14 +923,14 @@ test.describe( await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.TabsLayout.titleInput) .fill('Test tab 2'); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // clear the title input to simulate no title and click away to trigger onBlur await dashboardPage.getByGrafanaSelector(selectors.components.Tab.title('Test tab 2')).click(); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.TabsLayout.titleInput) .fill(''); - await dashboardPage.getByGrafanaSelector(selectors.components.EditPaneHeader.backButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.components.Sidebar.closePane).click(); // title should be set to a default name + 1 to avoid duplicates await expect(dashboardPage.getByGrafanaSelector(selectors.components.Tab.title('New tab 1'))).toBeVisible(); diff --git a/e2e-playwright/dashboard-new-layouts/dashboard-outline.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboard-outline.spec.ts index ef38379f006..00a84a188a5 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboard-outline.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboard-outline.spec.ts @@ -21,6 +21,7 @@ test.describe( const dashboardPage = await gotoDashboardPage({ uid: PAGE_UNDER_TEST }); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.outlineButton).click(); // Should be able to click Variables item in outline to see add variable button await dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.Outline.item('Variables')).click(); @@ -28,6 +29,8 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.addVariableButton) ).toBeVisible(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.outlineButton).click(); + // Clicking a panel should scroll that panel in view await expect(page.getByText('Dashboard panel 48')).toBeHidden(); await dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.Outline.item('Panel #48')).click(); diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts index ffea8243a2f..c2af96349cf 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts @@ -22,6 +22,9 @@ test.describe( const dashboardPage = await gotoDashboardPage({ uid: PAGE_UNDER_TEST }); await expect(page.getByText(DASHBOARD_NAME)).toBeVisible(); + const undockButton = page.getByRole('button', { name: 'Undock menu' }); + await undockButton.click(); + await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); await page.evaluate(() => { diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts index 24c61e5e960..4715dfc7128 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts @@ -199,6 +199,7 @@ test.describe( .click(); // Open the modal editor in the side pane + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.outlineButton).click(); await dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.Outline.node('Variables')).click(); await dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.Outline.item('foo')).click(); await openModal(dashboardPage, selectors); diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-panel-layouts.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-panel-layouts.spec.ts index 087551a55cd..66d634eefa7 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-panel-layouts.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-panel-layouts.spec.ts @@ -32,9 +32,9 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toHaveCount(3); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await page.getByLabel('Expand Panel layout category').click(); - - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); await expect( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) @@ -50,6 +50,7 @@ test.describe( ).toHaveCount(3); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await checkAutoGridLayoutInputs(dashboardPage, selectors); }); @@ -63,9 +64,10 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toHaveCount(3); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await page.getByLabel('Expand Panel layout category').click(); - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); // Get initial positions - standard width should have panels on different rows const firstPanelTop = await getPanelTop(dashboardPage, selectors); @@ -98,6 +100,7 @@ test.describe( await page.reload(); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await expect( dashboardPage.getByGrafanaSelector( @@ -123,9 +126,10 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toHaveCount(3); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await page.getByLabel('Expand Panel layout category').click(); - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.minColumnWidth) @@ -134,7 +138,7 @@ test.describe( await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.customMinColumnWidth) - .fill('900'); + .fill('1100'); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.customMinColumnWidth) .blur(); @@ -148,12 +152,13 @@ test.describe( await verifyPanelsStackedVertically(dashboardPage, selectors); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await expect( dashboardPage.getByGrafanaSelector( selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.customMinColumnWidth ) - ).toHaveValue('900'); + ).toHaveValue('1100'); await verifyPanelsStackedVertically(dashboardPage, selectors); @@ -180,9 +185,9 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toHaveCount(3); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await page.getByLabel('Expand Panel layout category').click(); - - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.maxColumns) @@ -198,6 +203,7 @@ test.describe( await verifyPanelsStackedVertically(dashboardPage, selectors); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await expect( dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.maxColumns) @@ -215,9 +221,9 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toHaveCount(3); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await page.getByLabel('Expand Panel layout category').click(); - - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); const regularRowHeight = await getPanelHeight(dashboardPage, selectors); @@ -250,6 +256,7 @@ test.describe( }).toPass(); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await expect( dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.rowHeight) @@ -270,9 +277,9 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toHaveCount(3); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await page.getByLabel('Expand Panel layout category').click(); - - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); const regularRowHeight = await getPanelHeight(dashboardPage, selectors); @@ -303,6 +310,7 @@ test.describe( }).toPass(); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await expect( dashboardPage.getByGrafanaSelector( @@ -327,9 +335,9 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toHaveCount(3); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await page.getByLabel('Expand Panel layout category').click(); - - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); // Set narrow column width first to ensure panels fit horizontally await dashboardPage @@ -357,6 +365,7 @@ test.describe( }).toPass(); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await expect( dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.AutoGridLayout.fillScreen) diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-repeats-auto-grid.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-repeats-auto-grid.spec.ts index 28ca51e3b28..7cf22108e1e 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-repeats-auto-grid.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-repeats-auto-grid.spec.ts @@ -37,9 +37,10 @@ test.describe( }, () => { test('can enable repeats', async ({ dashboardPage, selectors, page }) => { - await importTestDashboard(page, selectors, 'Auto grid repeats - add repeats'); + await importTestDashboard(page, selectors, 'Auto-grid repeats - add repeats'); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await switchToAutoGrid(page); @@ -70,11 +71,12 @@ test.describe( await importTestDashboard( page, selectors, - 'Auto grid repeats - update on variable change', + 'Auto-grid repeats - update on variable change', JSON.stringify(testV2DashWithRepeats) ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await switchToAutoGrid(page); await saveDashboard(dashboardPage, page, selectors); @@ -113,6 +115,7 @@ test.describe( ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await switchToAutoGrid(page); @@ -138,11 +141,13 @@ test.describe( await importTestDashboard( page, selectors, - 'Auto grid repeats - update through panel editor', + 'Auto-grid repeats - update through panel editor', JSON.stringify(testV2DashWithRepeats) ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); + await switchToAutoGrid(page); await saveDashboard(dashboardPage, page, selectors); await page.reload(); @@ -202,11 +207,13 @@ test.describe( await importTestDashboard( page, selectors, - 'Auto grid repeats - update through directly loaded panel editor', + 'Auto-grid repeats - update through directly loaded panel editor', JSON.stringify(testV2DashWithRepeats) ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); + await switchToAutoGrid(page); await saveDashboard(dashboardPage, page, selectors); @@ -257,11 +264,12 @@ test.describe( await importTestDashboard( page, selectors, - 'Auto grid repeats - move repeated panels', + 'Auto-grid repeats - move repeated panels', JSON.stringify(testV2DashWithRepeats) ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); await switchToAutoGrid(page); @@ -304,11 +312,13 @@ test.describe( await importTestDashboard( page, selectors, - 'Auto grid repeats - move repeated panels', + 'Auto-grid repeats - move repeated panels 2', JSON.stringify(testV2DashWithRepeats) ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); + await switchToAutoGrid(page); await saveDashboard(dashboardPage, page, selectors); await page.reload(); @@ -332,9 +342,7 @@ test.describe( const repeatedPanelUrl = page.url(); - await dashboardPage - .getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.backToDashboardButton) - .click(); + await page.keyboard.press('Escape'); await dashboardPage .getByGrafanaSelector(selectors.components.Panels.Panel.title(`${repeatTitleBase}${repeatOptions.at(0)}`)) @@ -367,11 +375,13 @@ test.describe( await importTestDashboard( page, selectors, - 'Auto grid repeats - view embedded repeated panel', + 'Auto-grid repeats - view embedded repeated panel', JSON.stringify(testV2DashWithRepeats) ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); + await switchToAutoGrid(page); await saveDashboard(dashboardPage, page, selectors); await page.reload(); @@ -393,11 +403,13 @@ test.describe( await importTestDashboard( page, selectors, - 'Auto grid repeats - remove repeats', + 'Auto-grid repeats - remove repeats', JSON.stringify(testV2DashWithRepeats) ); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); + await switchToAutoGrid(page); await saveDashboard(dashboardPage, page, selectors); await page.reload(); @@ -453,5 +465,5 @@ test.describe( async function switchToAutoGrid(page: Page) { await page.getByLabel('Expand Panel layout category').click(); - await page.getByLabel('Auto grid').click(); + await page.getByLabel('layout-selection-option-Auto grid').click(); } diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-repeats-custom-grid.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-repeats-custom-grid.spec.ts index 2d235571f97..8cc1f552377 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-repeats-custom-grid.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-repeats-custom-grid.spec.ts @@ -303,9 +303,7 @@ test.describe( const repeatedPanelUrl = page.url(); - await dashboardPage - .getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.backToDashboardButton) - .click(); + await page.keyboard.press('Escape'); await dashboardPage .getByGrafanaSelector(selectors.components.Panels.Panel.title(`${repeatTitleBase}${repeatOptions.at(0)}`)) diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-repeats-tabs-layout.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-repeats-tabs-layout.spec.ts index 100ec864b0d..f78bbc5e08e 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-repeats-tabs-layout.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-repeats-tabs-layout.spec.ts @@ -316,9 +316,7 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('New panel')) ).toBeVisible(); - await dashboardPage - .getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.backToDashboardButton) - .click(); + await page.keyboard.press('Escape'); // repeated panel in original tab repeat await dashboardPage @@ -341,9 +339,7 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Tab 1 - Row 2 - Panel repeat 2')) ).toBeVisible(); - await dashboardPage - .getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.backToDashboardButton) - .click(); + await page.keyboard.press('Escape'); // repeated panel in repeated tab await dashboardPage diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-title-description.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-title-description.spec.ts index 67dc17f648b..d9a82364a70 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-title-description.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-title-description.spec.ts @@ -21,11 +21,7 @@ test.describe( const dashboardPage = await gotoDashboardPage({ uid: PAGE_UNDER_TEST }); await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); - - // Check that current dashboard title is visible in breadcrumb - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Breadcrumbs.breadcrumb('Annotation filtering')) - ).toBeVisible(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.optionsButton).click(); const titleInput = page.locator('[aria-label="dashboard-options Title field property editor"] input'); await expect(titleInput).toHaveValue('Annotation filtering'); diff --git a/e2e-playwright/dashboard-new-layouts/utils.ts b/e2e-playwright/dashboard-new-layouts/utils.ts index da629ac8141..c1e00e1a67b 100644 --- a/e2e-playwright/dashboard-new-layouts/utils.ts +++ b/e2e-playwright/dashboard-new-layouts/utils.ts @@ -48,6 +48,7 @@ export const flows = { }, async newEditPaneVariableClick(dashboardPage: DashboardPage, selectors: E2ESelectorGroups) { await dashboardPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.editButton).click(); + await dashboardPage.getByGrafanaSelector(selectors.pages.Dashboard.Sidebar.outlineButton).click(); await dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.Outline.item('Variables')).click(); await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.ElementEditPane.addVariableButton) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 2450d3721d7..fde5e53587d 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1907,11 +1907,6 @@ "count": 2 } }, - "public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.tsx": { - "@typescript-eslint/consistent-type-assertions": { - "count": 1 - } - }, "public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx": { "react-hooks/rules-of-hooks": { "count": 4 diff --git a/packages/grafana-e2e-selectors/src/selectors/components.ts b/packages/grafana-e2e-selectors/src/selectors/components.ts index e56318477c0..98fa6053d11 100644 --- a/packages/grafana-e2e-selectors/src/selectors/components.ts +++ b/packages/grafana-e2e-selectors/src/selectors/components.ts @@ -57,6 +57,11 @@ export const versionedComponents = { '12.1.0': 'data-testid DashboardEditPaneSplitter primary body', }, }, + Sidebar: { + closePane: { + '12.4.0': 'data-testid Sidebar close pane', + }, + }, EditPaneHeader: { deleteButton: { '12.1.0': 'data-testid EditPaneHeader delete panel', @@ -70,9 +75,6 @@ export const versionedComponents = { duplicate: { '12.1.0': 'data-testid EditPaneHeader duplicate', }, - backButton: { - '12.1.0': 'data-testid EditPaneHeader back', - }, }, TimePicker: { openButton: { diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index 96d033d5b14..79422ee8db9 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -183,6 +183,14 @@ export const versionedPages = { url: { [MIN_GRAFANA_VERSION]: (uid: string) => `/d/${uid}`, }, + Sidebar: { + optionsButton: { + '12.4.0': 'data-testid Dashboard Sidebar options button', + }, + outlineButton: { + '12.4.0': 'data-testid Dashboard Sidebar outline button', + }, + }, DashNav: { nav: { [MIN_GRAFANA_VERSION]: 'Dashboard navigation', diff --git a/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx b/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx index 2d9b407a1ad..8c1bf784e53 100644 --- a/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx +++ b/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx @@ -59,8 +59,9 @@ export function SiderbarToolbar({ children }: SiderbarToolbarProps) { {context.hasOpenPane && ( )} diff --git a/packages/grafana-ui/src/components/Sidebar/SidebarButton.tsx b/packages/grafana-ui/src/components/Sidebar/SidebarButton.tsx index f436d9630f6..9f5e0f20d71 100644 --- a/packages/grafana-ui/src/components/Sidebar/SidebarButton.tsx +++ b/packages/grafana-ui/src/components/Sidebar/SidebarButton.tsx @@ -1,5 +1,5 @@ import { css, cx } from '@emotion/css'; -import { useContext } from 'react'; +import React, { ButtonHTMLAttributes, useContext } from 'react'; import { GrafanaTheme2, IconName, isIconName } from '@grafana/data'; @@ -11,38 +11,48 @@ import { Tooltip } from '../Tooltip/Tooltip'; import { SidebarContext } from './useSidebar'; -export interface Props { +export interface Props extends ButtonHTMLAttributes { icon: IconName; active?: boolean; - onClick?: () => void; - title: string; tooltip?: string; + title: string; } -export function SidebarButton({ icon, active, onClick, title, tooltip }: Props) { - const styles = useStyles2(getStyles); - const context = useContext(SidebarContext); +export const SidebarButton = React.forwardRef( + ({ icon, active, onClick, title, tooltip, ...restProps }, ref) => { + const styles = useStyles2(getStyles); + const context = useContext(SidebarContext); - if (!context) { - throw new Error('Sidebar.Button must be used within a Sidebar component'); + if (!context) { + throw new Error('Sidebar.Button must be used within a Sidebar component'); + } + + const buttonClass = cx( + styles.button, + context.compact && styles.compact, + active && styles.active, + context.position === 'left' && styles.leftButton + ); + + return ( + + + + ); } +); - const buttonClass = cx( - styles.button, - context.compact && styles.compact, - active && styles.active, - context.position === 'left' && styles.leftButton - ); - - return ( - - - - ); -} +SidebarButton.displayName = 'SidebarButton'; function renderIcon(icon: IconName | React.ReactNode, compact?: boolean) { if (!icon) { diff --git a/packages/grafana-ui/src/components/Sidebar/SidebarPaneHeader.tsx b/packages/grafana-ui/src/components/Sidebar/SidebarPaneHeader.tsx index 75393777879..42fe30b083c 100644 --- a/packages/grafana-ui/src/components/Sidebar/SidebarPaneHeader.tsx +++ b/packages/grafana-ui/src/components/Sidebar/SidebarPaneHeader.tsx @@ -2,6 +2,7 @@ import { css } from '@emotion/css'; import { ReactNode } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; +import { selectors } from '@grafana/e2e-selectors'; import { t } from '@grafana/i18n'; import { useStyles2 } from '../../themes/ThemeContext'; @@ -27,6 +28,7 @@ export function SidebarPaneHeader({ children, onClose, title }: Props) { onClick={onClose} aria-label={t('grafana-ui.sidebar.close', 'Close')} tooltip={t('grafana-ui.sidebar.close', 'Close')} + data-testid={selectors.components.Sidebar.closePane} /> )} diff --git a/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx b/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx index 03d04b2f81a..1d51b3fdd79 100644 --- a/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx +++ b/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx @@ -16,7 +16,7 @@ export interface SidebarContextValue { bottomMargin: number; edgeMargin: number; contentMargin: number; - onDockChange: () => void; + onToggleDock: () => void; onResize: (diff: number) => void; } @@ -56,7 +56,7 @@ export function useSidebar({ // Used to accumulate drag distance to know when to change compact mode const [_, setCompactDrag] = React.useState(0); - const onDockChange = useCallback(() => setIsDocked((prev) => !prev), []); + const onToggleDock = useCallback(() => setIsDocked((prev) => !prev), []); const prop = position === 'right' ? 'paddingRight' : 'paddingLeft'; const toolbarWidth = @@ -98,7 +98,7 @@ export function useSidebar({ return { isDocked, - onDockChange, + onToggleDock, onResize, outerWrapperProps, position, diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardEditPane.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardEditPane.tsx index 5a1b3624a2b..64f0c774ea8 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardEditPane.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardEditPane.tsx @@ -26,8 +26,12 @@ export interface DashboardEditPaneState extends SceneObjectState { undoStack: DashboardEditActionEventPayload[]; redoStack: DashboardEditActionEventPayload[]; + openPane?: DashboardSidebarPaneName; + isDocked?: boolean; } +export type DashboardSidebarPaneName = 'element' | 'outline' | 'filters'; + export class DashboardEditPane extends SceneObjectBase { public constructor() { super({ @@ -192,6 +196,7 @@ export class DashboardEditPane extends SceneObjectBase { this.setState({ selectionContext: { ...this.state.selectionContext, selected: [], enabled: false }, selection: undefined, + openPane: this.state.openPane === 'element' ? undefined : this.state.openPane, }); } @@ -227,7 +232,6 @@ export class DashboardEditPane extends SceneObjectBase { } const elementSelection = this.state.selection ?? new ElementSelection([[id, obj.getRef()]]); - const { selection, contextItems: selected } = elementSelection.getStateWithValue(id, obj, !!multi); this.updateSelection(new ElementSelection(selection), selected); @@ -255,17 +259,58 @@ export class DashboardEditPane extends SceneObjectBase { document.activeElement.blur(); } - this.setState({ selection, selectionContext: { ...this.state.selectionContext, selected } }); + this.setState({ + selection, + selectionContext: { ...this.state.selectionContext, selected }, + openPane: selection ? 'element' : undefined, + }); } - public clearSelection() { + /** + * @param force If force = true it will clear selection even when docked + * @returns + */ + public clearSelection(force = false) { if (!this.state.selection) { return; } + // If we are docked then clearing selection should select dashboard itself + // Unless the user explicitly closes pane + if (this.state.isDocked && !force) { + const obj = this.state.selection?.getFirstObject(); + const dashboard = getDashboardSceneFor(this); + if (obj !== dashboard) { + this.selectObject(dashboard, dashboard.state.key!); + } + return; + } + this.updateSelection(undefined, []); } + public openPane(openPane: DashboardSidebarPaneName) { + if (this.state.selection) { + this.clearSelection(true); + } + + if (openPane === this.state.openPane) { + this.setState({ openPane: undefined }); + } else { + this.setState({ openPane }); + } + } + + public closePane() { + if (this.state.selection) { + this.clearSelection(true); + } + + if (this.state.openPane) { + this.setState({ openPane: undefined }); + } + } + private newObjectAddedToCanvas(obj: SceneObject) { this.selectObject(obj, obj.state.key!); this.state.selection?.markAsNewElement(); diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.test.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.test.tsx index 78192a0c8fe..41c81222661 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.test.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.test.tsx @@ -1,18 +1,17 @@ -import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; +import { act, screen } from '@testing-library/react'; +import { render } from 'test/test-utils'; import { getPanelPlugin } from '@grafana/data/test'; import { selectors } from '@grafana/e2e-selectors'; -import { setPluginImportUtils } from '@grafana/runtime'; +import { setPluginImportUtils, config } from '@grafana/runtime'; import { SceneGridLayout, SceneTimeRange, SceneVariableSet, VizPanel } from '@grafana/scenes'; import { DashboardScene } from '../scene/DashboardScene'; import { DashboardGridItem } from '../scene/layout-default/DashboardGridItem'; import { DefaultGridLayoutManager } from '../scene/layout-default/DefaultGridLayoutManager'; -import { DashboardInteractions } from '../utils/interactions'; import { activateFullSceneTree } from '../utils/test-utils'; -import { DashboardEditPaneRenderer } from './DashboardEditPaneRenderer'; +import { DashboardEditPaneSplitter } from './DashboardEditPaneSplitter'; setPluginImportUtils({ importPanelPlugin: (id: string) => Promise.resolve(getPanelPlugin({})), @@ -26,14 +25,9 @@ jest.mock('../utils/interactions', () => ({ }, })); -jest.mock('react-router-dom-v5-compat', () => ({ - ...jest.requireActual('react-router-dom-v5-compat'), - useLocation: () => ({ - pathname: '/dashboard/test', - search: '', - hash: '', - state: null, - }), +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + useChromeHeaderHeight: jest.fn().mockReturnValue(80), })); export function buildTestScene() { @@ -47,26 +41,43 @@ export function buildTestScene() { }), }), }); - activateFullSceneTree(testScene); return testScene; } describe('DashboardEditPaneRenderer', () => { - describe('outline interactions tracking', () => { - it('should call DashboardInteractions.outlineClicked when clicking on dashboard outline', async () => { - const user = userEvent.setup(); - const scene = buildTestScene(); - render( - {}} - /> - ); - const outlineButton = screen.getByTestId(selectors.components.PanelEditor.Outline.section); - await user.click(outlineButton); + config.featureToggles.dashboardNewLayouts = true; - expect(DashboardInteractions.dashboardOutlineClicked).toHaveBeenCalled(); - }); + it('Should render sidebar', async () => { + const scene = buildTestScene(); + + act(() => activateFullSceneTree(scene)); + + render(); + + expect(await screen.findByTestId(selectors.pages.Dashboard.Sidebar.outlineButton)).toBeInTheDocument(); }); + + it('Should sync sidebar docked state with edit pane state', async () => { + const scene = buildTestScene(); + render(); + + act(() => screen.getByLabelText('Outline').click()); + + expect(await screen.findByTestId('sidebar-dock-toggle')).toBeInTheDocument(); + + act(() => screen.getByTestId('sidebar-dock-toggle').click()); + + expect(scene.state.editPane.state.isDocked).toBe(true); + }); + + // describe('outline interactions tracking', () => { + // it('should call DashboardInteractions.outlineClicked when clicking on dashboard outline', async () => { + // const user = userEvent.setup(); + // const scene = buildTestScene(); + // render(); + // const outlineButton = screen.getByTestId(selectors.components.PanelEditor.Outline.section); + // await user.click(outlineButton); + // expect(DashboardInteractions.dashboardOutlineClicked).toHaveBeenCalled(); + // }); + // }); }); diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.tsx index 9ca7fbab86e..80ec361476a 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneRenderer.tsx @@ -1,211 +1,160 @@ -import { css, cx } from '@emotion/css'; -import { Resizable } from 're-resizable'; -import { useLocalStorage } from 'react-use'; +import { useMemo } from 'react'; -import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { Trans, t } from '@grafana/i18n'; +import { t } from '@grafana/i18n'; +import { config } from '@grafana/runtime'; import { useSceneObjectState } from '@grafana/scenes'; -import { useStyles2, useSplitter, ToolbarButton, ScrollContainer, Text, Icon, clearButtonStyles } from '@grafana/ui'; +import { Sidebar } from '@grafana/ui'; +import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; -import { DashboardInteractions } from '../utils/interactions'; +import { DashboardScene } from '../scene/DashboardScene'; +import { onOpenSnapshotOriginalDashboard } from '../scene/GoToSnapshotOriginButton'; +import { ManagedDashboardNavBarBadge } from '../scene/ManagedDashboardNavBarBadge'; +import { ToolbarActionProps } from '../scene/new-toolbar/types'; +import { dynamicDashNavActions } from '../utils/registerDynamicDashNavAction'; import { DashboardEditPane } from './DashboardEditPane'; +import { ShareExportDashboardButton } from './DashboardExportButton'; import { DashboardOutline } from './DashboardOutline'; import { ElementEditPane } from './ElementEditPane'; -import { useEditableElement } from './useEditableElement'; export interface Props { editPane: DashboardEditPane; - isEditPaneCollapsed: boolean; - openOverlay?: boolean; - onToggleCollapse: () => void; + dashboard: DashboardScene; + isDocked?: boolean; } /** * Making the EditPane rendering completely standalone (not using editPane.Component) in order to pass custom react props */ -export function DashboardEditPaneRenderer({ editPane, isEditPaneCollapsed, onToggleCollapse, openOverlay }: Props) { - const { selection } = useSceneObjectState(editPane, { shouldActivateOrKeepAlive: true }); - const styles = useStyles2(getStyles); - const clearButton = useStyles2(clearButtonStyles); - const editableElement = useEditableElement(selection, editPane); +export function DashboardEditPaneRenderer({ editPane, dashboard, isDocked }: Props) { + const { selection, openPane } = useSceneObjectState(editPane, { shouldActivateOrKeepAlive: true }); + const { isEditing, meta, uid } = dashboard.useState(); + const hasUid = Boolean(uid); const selectedObject = selection?.getFirstObject(); - const isNewElement = selection?.isNewElement() ?? false; - const [outlineCollapsed, setOutlineCollapsed] = useLocalStorage( - 'grafana.dashboard.edit-pane.outline.collapsed', - false - ); - const [outlinePaneSize = 0.4, setOutlinePaneSize] = useLocalStorage('grafana.dashboard.edit-pane.outline.size', 0.4); - // splitter for template and payload editor - const splitter = useSplitter({ - direction: 'column', - handleSize: 'sm', - // if Grafana Alertmanager, split 50/50, otherwise 100/0 because there is no payload editor - initialSize: 1 - outlinePaneSize, - dragPosition: 'middle', - onSizeChanged: (size) => { - setOutlinePaneSize(1 - size); - }, - }); + const editableElement = useMemo(() => { + if (selection) { + return selection.createSelectionElement(); + } - if (!editableElement) { - return null; - } - - if (isEditPaneCollapsed) { - return ( - <> -
- -
- - {openOverlay && ( - - - - )} - - ); - } - - if (outlineCollapsed) { - splitter.primaryProps.style.flexGrow = 1; - splitter.primaryProps.style.minHeight = 'unset'; - splitter.secondaryProps.style.flexGrow = 0; - splitter.secondaryProps.style.minHeight = 'min-content'; - } else { - splitter.primaryProps.style.minHeight = 'unset'; - splitter.secondaryProps.style.minHeight = 'unset'; - } + return undefined; + }, [selection]); return ( -
-
-
+ <> + {editableElement && ( + -
-
-
- - {!outlineCollapsed && ( -
- - - -
- )} -
-
-
+ + )} + {openPane === 'outline' && ( + + + + )} + + {isEditing && ( + <> + {config.featureToggles.dashboardUndoRedo && ( + <> + + + + )} + editPane.selectObject(dashboard, dashboard.state.key!)} + title={t('dashboard.sidebar.dashboard-options.title', 'Options')} + tooltip={t('dashboard.sidebar.dashboard-options.tooltip', 'Dashboard options')} + data-testid={selectors.pages.Dashboard.Sidebar.optionsButton} + active={selectedObject === dashboard ? true : false} + /> + dashboard.openV2SchemaEditor()} + /> + + + )} + {hasUid && } + editPane.openPane('outline')} + title={t('dashboard.sidebar.outline.title', 'Outline')} + tooltip={t('dashboard.sidebar.outline.tooltip', 'Content outline')} + data-testid={selectors.pages.Dashboard.Sidebar.outlineButton} + active={openPane === 'outline'} + > + {dashboard.isManaged() && Boolean(meta.canEdit) && } + {renderEnterpriseItems()} + {Boolean(meta.isSnapshot) && ( + onOpenSnapshotOriginalDashboard(dashboard.getSnapshotUrl())} + /> + )} + + ); } -function getStyles(theme: GrafanaTheme2) { - return { - wrapper: css({ - display: 'flex', - flexDirection: 'column', - flex: '1 1 0', - marginTop: theme.spacing(2), - borderLeft: `1px solid ${theme.colors.border.weak}`, - borderTop: `1px solid ${theme.colors.border.weak}`, - background: theme.colors.background.primary, - borderTopLeftRadius: theme.shape.radius.default, - }), - overlayWrapper: css({ - right: 0, - bottom: 0, - top: theme.spacing(2), - position: 'absolute !important' as 'absolute', - background: theme.colors.background.primary, - borderLeft: `1px solid ${theme.colors.border.weak}`, - borderTop: `1px solid ${theme.colors.border.weak}`, - boxShadow: theme.shadows.z3, - zIndex: theme.zIndex.navbarFixed, - flexGrow: 1, - }), - paneContent: css({ - overflow: 'hidden', - display: 'flex', - flexDirection: 'column', - }), - rotate180: css({ - rotate: '180deg', - }), - tabsbar: css({ - padding: theme.spacing(0, 1), - margin: theme.spacing(0.5, 0), - }), - expandOptionsWrapper: css({ - display: 'flex', - flexDirection: 'column', - padding: theme.spacing(2, 1, 2, 0), - }), - splitter: css({ - '&::after': { - background: 'transparent', - transform: 'unset', - width: '100%', - height: '1px', - top: '100%', - left: '0', - }, - }), - outlineCollapseButton: css({ - display: 'flex', - padding: theme.spacing(0.5, 2), - gap: theme.spacing(1), - justifyContent: 'space-between', - alignItems: 'center', - background: theme.colors.background.secondary, +function renderEnterpriseItems() { + const dashboard = getDashboardSrv().getCurrent()!; + const showProps = { dashboard }; - '&:hover': { - background: theme.colors.action.hover, - }, - }), - outlineContainer: css({ - display: 'flex', - flexDirection: 'column', - flexGrow: 1, - overflow: 'hidden', - }), - }; + return dynamicDashNavActions.right.map((action, index) => { + if (action.show(showProps)) { + const ActionComponent = action.component; + return ; + } + return null; + }); +} + +function UndoButton({ dashboard }: ToolbarActionProps) { + const editPane = dashboard.state.editPane; + const { undoStack } = editPane.useState(); + const undoAction = undoStack[undoStack.length - 1]; + const undoWord = t('dashboard.sidebar.undo', 'Undo'); + const tooltip = `${undoWord}${undoAction?.description ? ` ${undoAction.description}` : ''}`; + + return ( + editPane.undoAction()} + title={undoWord} + tooltip={tooltip} + /> + ); +} + +function RedoButton({ dashboard }: ToolbarActionProps) { + const editPane = dashboard.state.editPane; + const { redoStack } = editPane.useState(); + const redoAction = redoStack[redoStack.length - 1]; + const redoWord = t('dashboard.sidebar.redo', 'Redo'); + const tooltip = `${redoWord}${redoAction?.description ? ` ${redoAction.description}` : ''}`; + + return ( + editPane.redoAction()} + /> + ); } diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx index 9ede14fe11d..ce7df7a460b 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx @@ -1,19 +1,22 @@ import { css, cx } from '@emotion/css'; -import React, { CSSProperties, useEffect } from 'react'; +import React, { useEffect } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { config, useChromeHeaderHeight } from '@grafana/runtime'; import { useSceneObjectState } from '@grafana/scenes'; -import { ElementSelectionContext, useStyles2 } from '@grafana/ui'; +import { ElementSelectionContext, useSidebar, useStyles2, Sidebar } from '@grafana/ui'; +import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate'; import NativeScrollbar, { DivScrollElement } from 'app/core/components/NativeScrollbar'; +import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; -import { useSnappingSplitter } from '../panel-edit/splitter/useSnappingSplitter'; import { DashboardScene } from '../scene/DashboardScene'; import { NavToolbarActions } from '../scene/NavToolbarActions'; +import { PublicDashboardBadge } from '../scene/new-toolbar/actions/PublicDashboardBadge'; +import { StarButton } from '../scene/new-toolbar/actions/StarButton'; +import { dynamicDashNavActions } from '../utils/registerDynamicDashNavAction'; import { DashboardEditPaneRenderer } from './DashboardEditPaneRenderer'; -import { useEditPaneCollapsed } from './shared'; interface Props { dashboard: DashboardScene; @@ -26,7 +29,10 @@ export function DashboardEditPaneSplitter({ dashboard, isEditing, body, controls const headerHeight = useChromeHeaderHeight(); const { editPane } = dashboard.state; const styles = useStyles2(getStyles, headerHeight ?? 0); - const [isCollapsed, setIsCollapsed] = useEditPaneCollapsed(); + const hasUid = Boolean(dashboard.state.uid); + const canStar = Boolean(dashboard.state.meta.canStar); + + //const [isCollapsed, setIsCollapsed] = useEditPaneCollapsed(); if (!config.featureToggles.dashboardNewLayouts) { return ( @@ -40,21 +46,6 @@ export function DashboardEditPaneSplitter({ dashboard, isEditing, body, controls ); } - const { containerProps, primaryProps, secondaryProps, splitterProps, splitterState, onToggleCollapse } = - useSnappingSplitter({ - direction: 'row', - dragPosition: 'end', - initialSize: 330, - handleSize: 'sm', - usePixels: true, - collapseBelowPixels: 250, - collapsed: isCollapsed, - }); - - useEffect(() => { - setIsCollapsed(splitterState.collapsed); - }, [splitterState.collapsed, setIsCollapsed]); - /** * Enable / disable selection based on dashboard isEditing state */ @@ -66,15 +57,7 @@ export function DashboardEditPaneSplitter({ dashboard, isEditing, body, controls } }, [isEditing, editPane]); - const { selectionContext } = useSceneObjectState(editPane, { shouldActivateOrKeepAlive: true }); - const containerStyle: CSSProperties = {}; - - if (!isEditing) { - primaryProps.style.flexGrow = 1; - primaryProps.style.width = '100%'; - primaryProps.style.minWidth = 'unset'; - containerStyle.overflow = 'unset'; - } + const { selectionContext, openPane } = useSceneObjectState(editPane, { shouldActivateOrKeepAlive: true }); const onBodyRef = (ref: HTMLDivElement | null) => { if (ref) { @@ -82,54 +65,73 @@ export function DashboardEditPaneSplitter({ dashboard, isEditing, body, controls } }; - return ( -
- -
{ - if (evt.shiftKey) { - return; - } + const sidebarContext = useSidebar({ + hasOpenPane: Boolean(openPane), + contentMargin: 1, + position: 'right', + }); - editPane.clearSelection(); - }} - > - -
{controls}
-
-
- {body} -
+ /** + * Sync docked state to editPane state + */ + useEffect(() => { + editPane.setState({ isDocked: sidebarContext.isDocked }); + }, [sidebarContext.isDocked, editPane]); + + const onClearSelection: React.PointerEventHandler = (evt) => { + if (evt.shiftKey) { + return; + } + + editPane.clearSelection(); + }; + + return ( +
+ + + {hasUid && canStar && } + {hasUid && canStar && } + {renderDynamicNavActions()} + + } + /> +
+ {controls} +
+
+
+ {body}
+ + +
- {isEditing && ( - <> -
-
- 0} - /> -
- - )}
); } +function renderDynamicNavActions() { + const dashboard = getDashboardSrv().getCurrent()!; + const showProps = { dashboard }; + + return dynamicDashNavActions.left.map((action, index) => { + if (action.show(showProps)) { + const ActionComponent = action.component; + return ; + } + return null; + }); +} + function getStyles(theme: GrafanaTheme2, headerHeight: number) { return { canvasWrappperOld: css({ @@ -138,22 +140,33 @@ function getStyles(theme: GrafanaTheme2, headerHeight: number) { flexDirection: 'column', flexGrow: 1, }), - canvasWithSplitter: css({ - overflow: 'unset', - display: 'flex', - flexDirection: 'column', - flexGrow: 1, - }), - canvasWithSplitterEditing: css({ - overflow: 'unset', - }), - bodyWrapper: css({ - label: 'body-wrapper', + container: css({ + label: 'container', display: 'flex', flexDirection: 'column', flexGrow: 1, position: 'relative', }), + bodyWrapper: css({ + label: 'body-wrapper', + display: 'flex', + flexDirection: 'row', + flexGrow: 1, + position: 'relative', + flex: '1 1 0', + overflow: 'hidden', + }), + bodyWithToolbar: css({ + display: 'flex', + flexDirection: 'column', + flexGrow: 1, + minHeight: 0, + overflow: 'auto', + scrollbarWidth: 'thin', + scrollbarGutter: 'stable', + // without top padding the fixed controls headers is rendered over the selection outline. + padding: theme.spacing(0.125, 1, 2, 2), + }), body: css({ label: 'body', display: 'flex', @@ -181,11 +194,6 @@ function getStyles(theme: GrafanaTheme2, headerHeight: number) { // borderLeft: `1px solid ${theme.colors.border.weak}`, // background: theme.colors.background.primary, }), - splitter: css({ - '&:after': { - display: 'none', - }, - }), controlsWrapperSticky: css({ [theme.breakpoints.up('md')]: { position: 'sticky', diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardEditableElement.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardEditableElement.tsx index 8c81d88b345..c68696e439d 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardEditableElement.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardEditableElement.tsx @@ -8,10 +8,9 @@ import { OptionsPaneItemDescriptor } from 'app/features/dashboard/components/Pan import { DashboardScene } from '../scene/DashboardScene'; import { useLayoutCategory } from '../scene/layouts-shared/DashboardLayoutSelector'; -import { EditSchemaV2Button } from '../scene/new-toolbar/actions/EditSchemaV2Button'; import { EditableDashboardElement, EditableDashboardElementInfo } from '../scene/types/EditableDashboardElement'; -import { dashboardEditActions, undoRedoWasClicked } from './shared'; +import { dashboardEditActions } from './shared'; function useEditPaneOptions( this: DashboardEditableElement, @@ -69,19 +68,16 @@ export class DashboardEditableElement implements EditableDashboardElement { public renderActions(): ReactNode { return ( - <> - - - + ); } } @@ -104,7 +100,7 @@ export function DashboardTitleInput({ dashboard, id }: { dashboard: DashboardSce }} onBlur={(e) => { const titleUnchanged = valueBeforeEdit.current === e.currentTarget.value; - const shouldSkip = titleUnchanged || undoRedoWasClicked(e); + const shouldSkip = titleUnchanged; if (shouldSkip) { return; } @@ -135,7 +131,7 @@ export function DashboardDescriptionInput({ dashboard, id }: { dashboard: Dashbo }} onBlur={(e) => { const descriptionUnchanged = valueBeforeEdit.current === e.currentTarget.value; - const shouldSkip = descriptionUnchanged || undoRedoWasClicked(e); + const shouldSkip = descriptionUnchanged; if (shouldSkip) { return; } diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardExportButton.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardExportButton.tsx new file mode 100644 index 00000000000..35d8afbdb4d --- /dev/null +++ b/public/app/features/dashboard-scene/edit-pane/DashboardExportButton.tsx @@ -0,0 +1,57 @@ +import { selectors } from '@grafana/e2e-selectors'; +import { t } from '@grafana/i18n'; +import { locationService } from '@grafana/runtime'; +import { Dropdown, Sidebar } from '@grafana/ui'; +import { appEvents } from 'app/core/app_events'; +import { getTrackingSource, shareDashboardType } from 'app/features/dashboard/components/ShareModal/utils'; +import { ShowConfirmModalEvent } from 'app/types/events'; + +import { DashboardScene } from '../scene/DashboardScene'; +import ExportMenu from '../sharing/ExportButton/ExportMenu'; +import { DashboardInteractions } from '../utils/interactions'; + +interface Props { + dashboard: DashboardScene; +} + +const newExportButtonSelector = selectors.pages.Dashboard.DashNav.NewExportButton; + +export function ShareExportDashboardButton({ dashboard }: Props) { + return ( + } placement="left-end"> + { + if (dashboard.state.isEditing && dashboard.state.isDirty) { + evt.preventDefault(); + evt.stopPropagation(); + + appEvents.publish( + new ShowConfirmModalEvent({ + title: t('dashboard.sidebar.export.unsaved-modal.title', 'Save changes to dashboard?'), + text: t( + 'dashboard.sidebar.export.unsaved-modal.text', + 'You have unsaved changes to this dashboard. You need to save them before you can share it.' + ), + icon: 'exclamation-triangle', + noText: t('common.discard', 'Discard'), + yesText: t('common.save', 'Save'), + yesButtonVariant: 'primary', + onConfirm: () => dashboard.openSaveDrawer({}), + }) + ); + } else { + locationService.partial({ shareView: shareDashboardType.export }); + + DashboardInteractions.sharingCategoryClicked({ + item: shareDashboardType.export, + shareResource: getTrackingSource(), + }); + } + }} + /> + + ); +} diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardOutline.test.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardOutline.test.tsx index 81d56cab57d..7b461551773 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardOutline.test.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardOutline.test.tsx @@ -101,7 +101,7 @@ describe('DashboardOutline', () => { render( - + ); // select Row lvl 1 diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardOutline.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardOutline.tsx index 48a9c738856..4ce6536ac5c 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, Stack, Text, useElementSelection, useStyles2 } from '@grafana/ui'; +import { Box, Icon, Sidebar, Stack, Text, useElementSelection, useStyles2 } from '@grafana/ui'; import { isRepeatCloneOrChildOf } from '../utils/clone'; import { DashboardInteractions } from '../utils/interactions'; @@ -17,28 +17,36 @@ import { useOutlineRename } from './useOutlineRename'; export interface Props { editPane: DashboardEditPane; + isEditing: boolean | undefined; } -export function DashboardOutline({ editPane }: Props) { +export function DashboardOutline({ editPane, isEditing }: Props) { const dashboard = getDashboardSceneFor(editPane); return ( - - - + <> + editPane.closePane()} + /> + + + + ); } interface DashboardOutlineNodeProps { sceneObject: SceneObject; editPane: DashboardEditPane; + isEditing: boolean | undefined; depth: number; index: number; } -function DashboardOutlineNode({ sceneObject, editPane, depth, index }: DashboardOutlineNodeProps) { +function DashboardOutlineNode({ sceneObject, editPane, isEditing, depth, index }: DashboardOutlineNodeProps) { const styles = useStyles2(getStyles); - const { key } = sceneObject.useState(); + const key = sceneObject.state.key; const [isCollapsed, setIsCollapsed] = useState(depth > 0); const { isSelected, onSelect } = useElementSelection(key); const isCloned = useMemo(() => isRepeatCloneOrChildOf(sceneObject), [sceneObject]); @@ -49,7 +57,7 @@ function DashboardOutlineNode({ sceneObject, editPane, depth, index }: Dashboard const children = editableElement.getOutlineChildren?.() ?? []; const elementInfo = editableElement.getEditableElementInfo(); const instanceName = elementInfo.instanceName === '' ? noTitleText : elementInfo.instanceName; - const outlineRename = useOutlineRename(editableElement); + const outlineRename = useOutlineRename(editableElement, isEditing); const isContainer = editableElement.getOutlineChildren ? true : false; const onNodeClicked = (e: React.MouseEvent) => { @@ -131,6 +139,7 @@ function DashboardOutlineNode({ sceneObject, editPane, depth, index }: Dashboard sceneObject={child} editPane={editPane} depth={depth + 1} + isEditing={isEditing} index={i} /> )) diff --git a/public/app/features/dashboard-scene/edit-pane/EditPaneHeader.tsx b/public/app/features/dashboard-scene/edit-pane/EditPaneHeader.tsx index b2a7a149715..ea14464fc64 100644 --- a/public/app/features/dashboard-scene/edit-pane/EditPaneHeader.tsx +++ b/public/app/features/dashboard-scene/edit-pane/EditPaneHeader.tsx @@ -1,9 +1,6 @@ -import { css } from '@emotion/css'; - -import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { t } from '@grafana/i18n'; -import { Button, Menu, Stack, Text, useStyles2, Dropdown, Icon, IconButton } from '@grafana/ui'; +import { Button, Menu, Stack, Dropdown, Icon, Sidebar } from '@grafana/ui'; import { trackDeleteDashboardElement } from 'app/features/dashboard-scene/utils/tracking'; import { EditableDashboardElement } from '../scene/types/EditableDashboardElement'; @@ -17,15 +14,11 @@ interface EditPaneHeaderProps { export function EditPaneHeader({ element, editPane }: EditPaneHeaderProps) { const elementInfo = element.getEditableElementInfo(); - const styles = useStyles2(getStyles); const onCopy = element.onCopy?.bind(element); const onDuplicate = element.onDuplicate?.bind(element); const onDelete = element.onDelete?.bind(element); const onConfirmDelete = element.onConfirmDelete?.bind(element); - // temporary simple solution, should select parent element - const onGoBack = () => editPane.clearSelection(); - const canGoBack = editPane.state.selection; const onDeleteElement = () => { if (onConfirmDelete) { @@ -37,20 +30,7 @@ export function EditPaneHeader({ element, editPane }: EditPaneHeaderProps) { }; return ( -
- - {canGoBack && ( - - )} - {elementInfo.typeName} - + editPane.closePane()}> {element.renderActions && element.renderActions()} {(onCopy || onDuplicate) && ( @@ -95,18 +75,6 @@ export function EditPaneHeader({ element, editPane }: EditPaneHeaderProps) { /> )} -
+ ); } - -function getStyles(theme: GrafanaTheme2) { - return { - wrapper: css({ - display: 'flex', - alignItems: 'center', - justifyContent: 'space-between', - padding: theme.spacing(1, 2), - borderBottom: `1px solid ${theme.colors.border.weak}`, - }), - }; -} diff --git a/public/app/features/dashboard-scene/edit-pane/shared.ts b/public/app/features/dashboard-scene/edit-pane/shared.ts index 6d9a2279563..39fd5fec9a7 100644 --- a/public/app/features/dashboard-scene/edit-pane/shared.ts +++ b/public/app/features/dashboard-scene/edit-pane/shared.ts @@ -14,7 +14,6 @@ import { import { DashboardScene } from '../scene/DashboardScene'; import { SceneGridRowEditableElement } from '../scene/layout-default/SceneGridRowEditableElement'; -import { redoButtonId, undoButtonID } from '../scene/new-toolbar/RightActions'; import { EditableDashboardElement, isEditableDashboardElement } from '../scene/types/EditableDashboardElement'; import { LocalVariableEditableElement } from '../settings/variables/LocalVariableEditableElement'; import { VariableAdd, VariableAddEditableElement } from '../settings/variables/VariableAddEditableElement'; @@ -299,7 +298,3 @@ function makeEditAction { - if (!selection) { - const dashboard = getDashboardSceneFor(editPane); - return new ElementSelection([[dashboard.state.uid!, dashboard.getRef()]]).createSelectionElement(); - } - - return selection.createSelectionElement(); - }, [selection, editPane]); -} diff --git a/public/app/features/dashboard-scene/edit-pane/useOutlineRename.tsx b/public/app/features/dashboard-scene/edit-pane/useOutlineRename.tsx index 81f3a4556f0..d87d83994eb 100644 --- a/public/app/features/dashboard-scene/edit-pane/useOutlineRename.tsx +++ b/public/app/features/dashboard-scene/edit-pane/useOutlineRename.tsx @@ -8,10 +8,14 @@ export interface OutlineRenameState { error?: string; } -export function useOutlineRename(editableElement: EditableDashboardElement) { +export function useOutlineRename(editableElement: EditableDashboardElement, isEditing: boolean | undefined) { const [state, setState] = useState({}); const onNameDoubleClicked = (evt: React.MouseEvent) => { + if (!isEditing) { + return; + } + if (!editableElement.onChangeName) { return; } diff --git a/public/app/features/dashboard-scene/scene/DashboardControls.tsx b/public/app/features/dashboard-scene/scene/DashboardControls.tsx index 5268ea4d865..da18ca17dd4 100644 --- a/public/app/features/dashboard-scene/scene/DashboardControls.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardControls.tsx @@ -2,6 +2,8 @@ import { css, cx } from '@emotion/css'; import { GrafanaTheme2, VariableHide } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; +import { Trans } from '@grafana/i18n'; +import { config } from '@grafana/runtime'; import { SceneObjectState, SceneObjectBase, @@ -15,7 +17,8 @@ import { SceneObjectUrlValues, CancelActivationHandler, } from '@grafana/scenes'; -import { Box, useStyles2 } from '@grafana/ui'; +import { Box, Button, useStyles2 } from '@grafana/ui'; +import { playlistSrv } from 'app/features/playlist/PlaylistSrv'; import { PanelEditControls } from '../panel-edit/PanelEditControls'; import { getDashboardSceneFor } from '../utils/utils'; @@ -25,6 +28,9 @@ import { DashboardDataLayerControls } from './DashboardDataLayerControls'; import { DashboardLinksControls } from './DashboardLinksControls'; import { DashboardScene } from './DashboardScene'; import { VariableControls } from './VariableControls'; +import { EditDashboardSwitch } from './new-toolbar/actions/EditDashboardSwitch'; +import { SaveDashboard } from './new-toolbar/actions/SaveDashboard'; +import { ShareDashboardButton } from './new-toolbar/actions/ShareDashboardButton'; export interface DashboardControlsState extends SceneObjectState { timePicker: SceneTimePicker; @@ -32,7 +38,7 @@ export interface DashboardControlsState extends SceneObjectState { hideTimeControls?: boolean; hideVariableControls?: boolean; hideLinksControls?: boolean; - // Hides the dashbaord-controls dropdown menu + // Hides the dashboard-controls dropdown menu hideDashboardControls?: boolean; } @@ -171,6 +177,7 @@ function DashboardControlsRenderer({ model }: SceneComponentProps )} {!hideDashboardControls && model.hasDashboardControls() && } + {config.featureToggles.dashboardNewLayouts && }
{!hideVariableControls && ( <> @@ -185,6 +192,37 @@ function DashboardControlsRenderer({ model }: SceneComponentProps + {showShareButton && } + {isEditing && } + {!isPlaying && canEditDashboard && } + {isPlaying && ( + + )} + + ); +} + function renderHiddenVariables(dashboard: DashboardScene) { const { variables } = sceneGraph.getVariables(dashboard).useState(); const renderAsHiddenVariables = variables.filter((v) => v.UNSAFE_renderAsHidden); diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index 1c75360a7b5..a542dd92826 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.tsx @@ -275,7 +275,7 @@ export class DashboardScene extends SceneObjectBase impleme this._initialUrlState = locationService.getLocation(); // Switch to edit mode - this.setState({ isEditing: true }); + this.setState({ isEditing: true, editable: true }); // Propagate change edit mode change to children this.state.body.editModeChanged?.(true); @@ -692,16 +692,16 @@ export class DashboardScene extends SceneObjectBase impleme canEditDashboard() { const { meta } = this.state; - return Boolean(meta.canEdit || meta.canMakeEditable || config.viewersCanEdit); + return !meta.isSnapshot && Boolean(meta.canEdit || meta.canMakeEditable || config.viewersCanEdit); } public getInitialSaveModel() { return this.serializer.initialSaveModel; } - public getSnapshotUrl = () => { - return this.serializer.getSnapshotUrl(); - }; + public getSnapshotUrl() { + return this.serializer.getSnapshotUrl() ?? ''; + } /** Hacky temp function until we refactor transformSaveModelToScene a bit */ setInitialSaveModel(model?: Dashboard, meta?: DashboardMeta, apiVersion?: string): void; diff --git a/public/app/features/dashboard-scene/scene/GoToSnapshotOriginButton.tsx b/public/app/features/dashboard-scene/scene/GoToSnapshotOriginButton.tsx index 2efcad5a256..80c8487f1c5 100644 --- a/public/app/features/dashboard-scene/scene/GoToSnapshotOriginButton.tsx +++ b/public/app/features/dashboard-scene/scene/GoToSnapshotOriginButton.tsx @@ -20,7 +20,7 @@ export function GoToSnapshotOriginButton(props: { originalURL: string }) { ); } -const onOpenSnapshotOriginalDashboard = (originalUrl: string) => { +export const onOpenSnapshotOriginalDashboard = (originalUrl: string) => { const relativeURL = originalUrl ?? ''; const sanitizedRelativeURL = textUtil.sanitizeUrl(relativeURL); try { diff --git a/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx b/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx index 68ccadaba13..959913c8f8f 100644 --- a/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx +++ b/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx @@ -153,9 +153,7 @@ export function ToolbarActions({ dashboard }: Props) { toolbarActions.push({ group: 'icon-actions', condition: meta.isSnapshot && !isEditing, - render: () => ( - - ), + render: () => , }); if (!isEditingPanel && !isEditing) { diff --git a/public/app/features/dashboard-scene/scene/new-toolbar/RightActions.tsx b/public/app/features/dashboard-scene/scene/new-toolbar/RightActions.tsx index 59dfc714669..2e2f979d51d 100644 --- a/public/app/features/dashboard-scene/scene/new-toolbar/RightActions.tsx +++ b/public/app/features/dashboard-scene/scene/new-toolbar/RightActions.tsx @@ -1,8 +1,7 @@ import { css } from '@emotion/css'; import { GrafanaTheme2 } from '@grafana/data'; -import { config } from '@grafana/runtime'; -import { ToolbarButton, ToolbarButtonRow, useStyles2 } from '@grafana/ui'; +import { ToolbarButtonRow, useStyles2 } from '@grafana/ui'; import { contextSrv } from 'app/core/services/context_srv'; import { playlistSrv } from 'app/features/playlist/PlaylistSrv'; @@ -11,45 +10,35 @@ import { isLibraryPanel } from '../../utils/utils'; import { DashboardScene } from '../DashboardScene'; import { BackToDashboardButton } from './actions/BackToDashboardButton'; -import { DashboardSettingsButton } from './actions/DashboardSettingsButton'; import { DiscardLibraryPanelButton } from './actions/DiscardLibraryPanelButton'; import { DiscardPanelButton } from './actions/DiscardPanelButton'; -import { EditDashboardSwitch } from './actions/EditDashboardSwitch'; -import { ExportDashboardButton } from './actions/ExportDashboardButton'; import { MakeDashboardEditableButton } from './actions/MakeDashboardEditableButton'; import { PlayListNextButton } from './actions/PlayListNextButton'; import { PlayListPreviousButton } from './actions/PlayListPreviousButton'; import { PlayListStopButton } from './actions/PlayListStopButton'; import { SaveDashboard } from './actions/SaveDashboard'; import { SaveLibraryPanelButton } from './actions/SaveLibraryPanelButton'; -import { ShareDashboardButton } from './actions/ShareDashboardButton'; import { UnlinkLibraryPanelButton } from './actions/UnlinkLibraryPanelButton'; -import { ToolbarActionProps } from './types'; import { getDynamicActions, renderActionElements } from './utils'; export const RightActions = ({ dashboard }: { dashboard: DashboardScene }) => { - const { editPanel, editable, editview, isEditing, uid, meta, viewPanel } = dashboard.useState(); + const { editPanel, editable, editview, isEditing, meta, viewPanel } = dashboard.useState(); const { isPlaying } = playlistSrv.useState(); const styles = useStyles2(getStyles); const isEditable = Boolean(editable); const canSave = Boolean(meta.canSave); - const hasUid = Boolean(uid); const isEditingDashboard = Boolean(isEditing); const hasEditView = Boolean(editview); const isEditingPanel = Boolean(editPanel); const isViewingPanel = Boolean(viewPanel); const isEditingLibraryPanel = isEditingPanel && isLibraryPanel(editPanel!.state.panelRef.resolve()); const isShowingDashboard = !hasEditView && !isViewingPanel && !isEditingPanel; - const isEditingAndShowingDashboard = isEditingDashboard && isShowingDashboard; - const isSnapshot = Boolean(meta.isSnapshot); const canSaveInFolder = contextSrv.hasEditPermissionInFolders; const canEditDashboard = dashboard.canEditDashboard(); const showPanelButtons = isEditingPanel && !hasEditView && !isViewingPanel; const showPlayButtons = isPlaying && isShowingDashboard && !isEditingDashboard; - const showShareButton = hasUid && !isSnapshot && !isPlaying && !isEditingPanel; - const showUndoRedoButtons = isEditingAndShowingDashboard && !!config.featureToggles.dashboardUndoRedo; return ( @@ -106,28 +95,10 @@ export const RightActions = ({ dashboard }: { dashboard: DashboardScene }) => { group: 'panel', condition: showPanelButtons && isEditingLibraryPanel, }, - { - key: 'dashboard-undo', - component: UndoButton, - group: 'dashboard', - condition: showUndoRedoButtons, - }, - { - key: 'dashboard-redo', - component: RedoButton, - group: 'dashboard', - condition: showUndoRedoButtons, - }, - { - key: 'dashboard-settings', - component: DashboardSettingsButton, - group: 'dashboard', - condition: isEditingAndShowingDashboard && canEditDashboard, - }, { key: 'save-dashboard', component: SaveDashboard, - group: 'save-edit', + group: 'panel', condition: isEditingDashboard && !isEditingLibraryPanel && (canSave || canSaveInFolder), }, { @@ -136,31 +107,6 @@ export const RightActions = ({ dashboard }: { dashboard: DashboardScene }) => { group: 'save-edit', condition: !isEditing && canEditDashboard && !isViewingPanel && !isEditable && !isPlaying, }, - { - key: 'edit-dashboard-switch', - component: EditDashboardSwitch, - group: 'save-edit', - condition: - canEditDashboard && - !isEditingPanel && - !isEditingLibraryPanel && - !isViewingPanel && - isEditable && - !isPlaying && - !isEditingPanel, - }, - { - key: 'new-export-dashboard-button', - component: ExportDashboardButton, - group: 'export-share', - condition: showShareButton, - }, - { - key: 'new-share-dashboard-button', - component: ShareDashboardButton, - group: 'export-share', - condition: showShareButton, - }, ], dashboard )} @@ -168,42 +114,6 @@ export const RightActions = ({ dashboard }: { dashboard: DashboardScene }) => { ); }; -export const undoButtonID = 'undo-button'; -function UndoButton({ dashboard }: ToolbarActionProps) { - const editPane = dashboard.state.editPane; - const { undoStack } = editPane.useState(); - const undoAction = undoStack[undoStack.length - 1]; - const tooltip = `Undo${undoAction?.description ? ` '${undoAction.description}'` : ''}`; - - return ( - editPane.undoAction()} - tooltip={tooltip} - /> - ); -} - -export const redoButtonId = 'redo-button'; -function RedoButton({ dashboard }: ToolbarActionProps) { - const editPane = dashboard.state.editPane; - const { redoStack } = editPane.useState(); - const redoAction = redoStack[redoStack.length - 1]; - const tooltip = `Redo${redoAction?.description ? ` '${redoAction.description}'` : ''}`; - - return ( - editPane.redoAction()} - /> - ); -} - const getStyles = (theme: GrafanaTheme2) => ({ container: css({ paddingLeft: theme.spacing(0.5) }), }); diff --git a/public/app/features/dashboard-scene/scene/new-toolbar/actions/ExportDashboardButton.tsx b/public/app/features/dashboard-scene/scene/new-toolbar/actions/ExportDashboardButton.tsx deleted file mode 100644 index d8004d00b13..00000000000 --- a/public/app/features/dashboard-scene/scene/new-toolbar/actions/ExportDashboardButton.tsx +++ /dev/null @@ -1,39 +0,0 @@ -import { selectors as e2eSelectors } from '@grafana/e2e-selectors'; -import { t } from '@grafana/i18n'; -import { config, locationService } from '@grafana/runtime'; -import { getTrackingSource, shareDashboardType } from 'app/features/dashboard/components/ShareModal/utils'; - -import ExportMenu from '../../../sharing/ExportButton/ExportMenu'; -import { DashboardInteractions } from '../../../utils/interactions'; -import { ToolbarActionProps } from '../types'; - -import { ShareExportDashboardButton } from './ShareExportDashboardButton'; - -const newExportButtonSelector = e2eSelectors.pages.Dashboard.DashNav.NewExportButton; - -export const ExportDashboardButton = ({ dashboard }: ToolbarActionProps) => { - const buttonTooltip = config.featureToggles.kubernetesDashboards - ? t('dashboard.toolbar.new.export.tooltip.as-code', 'Export as code') - : t('dashboard.toolbar.new.export.tooltip.json', 'Export as JSON'); - - return ( - } - groupTestId={newExportButtonSelector.container} - buttonLabel={t('dashboard.toolbar.new.export.title', 'Export')} - buttonTooltip={buttonTooltip} - buttonTestId={newExportButtonSelector.container} - onButtonClick={() => { - locationService.partial({ shareView: shareDashboardType.export }); - - DashboardInteractions.sharingCategoryClicked({ - item: shareDashboardType.export, - shareResource: getTrackingSource(), - }); - }} - arrowLabel={t('dashboard.toolbar.new.export.arrow', 'Export')} - arrowTestId={newExportButtonSelector.arrowMenu} - dashboard={dashboard} - /> - ); -}; diff --git a/public/app/features/dashboard-scene/scene/new-toolbar/actions/OpenSnapshotOriginButton.tsx b/public/app/features/dashboard-scene/scene/new-toolbar/actions/OpenSnapshotOriginButton.tsx index c59d06a74ae..51d04d2272a 100644 --- a/public/app/features/dashboard-scene/scene/new-toolbar/actions/OpenSnapshotOriginButton.tsx +++ b/public/app/features/dashboard-scene/scene/new-toolbar/actions/OpenSnapshotOriginButton.tsx @@ -2,5 +2,5 @@ import { GoToSnapshotOriginButton } from '../../GoToSnapshotOriginButton'; import { ToolbarActionProps } from '../types'; export const OpenSnapshotOriginButton = ({ dashboard }: ToolbarActionProps) => ( - + ); diff --git a/public/app/features/dashboard-scene/scene/new-toolbar/actions/SaveDashboard.tsx b/public/app/features/dashboard-scene/scene/new-toolbar/actions/SaveDashboard.tsx index 0c33cf19b68..f2819e44c1c 100644 --- a/public/app/features/dashboard-scene/scene/new-toolbar/actions/SaveDashboard.tsx +++ b/public/app/features/dashboard-scene/scene/new-toolbar/actions/SaveDashboard.tsx @@ -6,10 +6,12 @@ import { contextSrv } from 'app/core/services/context_srv'; import { ToolbarActionProps } from '../types'; export const SaveDashboard = ({ dashboard }: ToolbarActionProps) => { - const { meta, isDirty, uid } = dashboard.state; + const { meta, isDirty, uid, editview, editPanel } = dashboard.state; const isNew = !Boolean(uid || dashboard.isManaged()); const isManaged = dashboard.isManaged(); + // In dashboard settings we still use the nav toolbar for a short while + const buttonSize = Boolean(editview) || editPanel ? 'sm' : 'md'; // if we only can save if (isNew) { @@ -17,7 +19,7 @@ export const SaveDashboard = ({ dashboard }: ToolbarActionProps) => { diff --git a/public/app/features/dashboard-scene/scene/new-toolbar/actions/ToolbarSwitch.tsx b/public/app/features/dashboard-scene/scene/new-toolbar/actions/ToolbarSwitch.tsx index 59262fc152e..0ec0232e985 100644 --- a/public/app/features/dashboard-scene/scene/new-toolbar/actions/ToolbarSwitch.tsx +++ b/public/app/features/dashboard-scene/scene/new-toolbar/actions/ToolbarSwitch.tsx @@ -43,7 +43,7 @@ export const ToolbarSwitch = ({ onClick={disabled ? undefined : onClick} >
- +
@@ -53,11 +53,11 @@ export const ToolbarSwitch = ({ const getStyles = (theme: GrafanaTheme2) => ({ container: css({ border: `1px solid ${theme.components.input.borderColor}`, - padding: theme.spacing(0.25), + padding: theme.spacing(0.5), backgroundColor: theme.components.input.background, borderRadius: theme.shape.radius.default, - width: theme.spacing(5.5), - height: theme.spacing(3), + width: theme.spacing(6.5), + height: theme.spacing(theme.components.height.md), cursor: 'pointer', display: 'flex', flexDirection: 'row', @@ -90,19 +90,19 @@ const getStyles = (theme: GrafanaTheme2) => ({ display: 'flex', alignItems: 'center', justifyContent: 'center', - width: theme.spacing(2.5), + width: theme.spacing(3.5), height: '100%', transform: 'translateX(0)', position: 'relative', borderRadius: styleMixins.getInternalRadius(theme, 2), - border: `1px solid ${theme.colors.secondary.border}`, + border: `1px solid ${theme.colors.border.weak}`, [theme.transitions.handleMotion('no-preference', 'reduce')]: { transition: 'all 0.2s ease-in-out', }, }), boxChecked: css({ - transform: `translateX(calc(100% - ${theme.spacing(0.25)}))`, + transform: `translateX(calc(100% - 14px))`, borderColor: 'transparent', }), }); diff --git a/public/app/features/dashboard-scene/settings/variables/VariableEditableElement.tsx b/public/app/features/dashboard-scene/settings/variables/VariableEditableElement.tsx index 78876f4ee55..00e072e4d6f 100644 --- a/public/app/features/dashboard-scene/settings/variables/VariableEditableElement.tsx +++ b/public/app/features/dashboard-scene/settings/variables/VariableEditableElement.tsx @@ -9,7 +9,7 @@ import { Input, TextArea, Button, Field, Box, Stack } from '@grafana/ui'; import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor'; import { OptionsPaneItemDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneItemDescriptor'; -import { dashboardEditActions, undoRedoWasClicked } from '../../edit-pane/shared'; +import { dashboardEditActions } from '../../edit-pane/shared'; import { useEditPaneInputAutoFocus } from '../../scene/layouts-shared/utils'; import { BulkActionElement } from '../../scene/types/BulkActionElement'; import { EditableDashboardElement, EditableDashboardElementInfo } from '../../scene/types/EditableDashboardElement'; @@ -161,7 +161,7 @@ function VariableNameInput({ variable, isNewElement }: { variable: SceneVariable onChange={onChange} onBlur={(e) => { const labelUnchanged = oldName.current === name; - const shouldSkip = labelUnchanged || undoRedoWasClicked(e); + const shouldSkip = labelUnchanged; if (nameError) { setNameError(undefined); @@ -200,7 +200,7 @@ function VariableLabelInput({ variable, id }: VariableInputProps) { onChange={(e) => variable.setState({ label: e.currentTarget.value })} onBlur={(e) => { const labelUnchanged = oldLabel.current === e.currentTarget.value; - const shouldSkip = labelUnchanged || undoRedoWasClicked(e); + const shouldSkip = labelUnchanged; if (shouldSkip) { return; @@ -232,7 +232,7 @@ function VariableDescriptionTextArea({ variable, id }: VariableInputProps) { onChange={(e) => variable.setState({ description: e.currentTarget.value })} onBlur={(e) => { const labelUnchanged = oldDescription.current === e.currentTarget.value; - const shouldSkip = labelUnchanged || undoRedoWasClicked(e); + const shouldSkip = labelUnchanged; if (shouldSkip) { return; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index e8d4b15d05f..dad607c0de0 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -4172,6 +4172,7 @@ "clear": "Clear", "collapse": "Collapse", "disabled": "Disabled", + "discard": "Discard", "edit": "Edit", "help": "Help", "loading": "Loading...", @@ -4789,7 +4790,6 @@ "variable": "{{type}} variable", "variable-set": "Variables" }, - "open": "Open options pane", "row": { "header": { "hide": "Hide", @@ -5140,6 +5140,7 @@ "title-matched_other": "Matched {{count}}/{{totalCount}} options" }, "outline": { + "pane-header": "Content outline", "repeated-item": "Repeat", "tree-item": { "empty": "(empty)", @@ -5351,6 +5352,32 @@ "share-public-dashboard-loader": { "loading-configuration": "Loading configuration" }, + "sidebar": { + "dashboard-options": { + "title": "Options", + "tooltip": "Dashboard options" + }, + "edit-schema": { + "title": "Code", + "tooltip": "Edit as code" + }, + "export": { + "title": "Export", + "unsaved-modal": { + "text": "You have unsaved changes to this dashboard. You need to save them before you can share it.", + "title": "Save changes to dashboard?" + } + }, + "outline": { + "title": "Outline", + "tooltip": "Content outline" + }, + "redo": "Redo", + "snapshot": { + "tooltip": "Open original dashboard" + }, + "undo": "Undo" + }, "solo-panel": { "loading-initializing-dashboard": "Loading & initializing dashboard", "title-not-found": "Panel with id {{panelId}} not found" @@ -5454,11 +5481,8 @@ "tooltip": "This dashboard was marked as read only" }, "export": { - "arrow": "Export", - "title": "Export", "tooltip": { - "as-code": "Export as code", - "json": "Export as JSON" + "as-code": "Export as code" } }, "more-save-options": "More save options", @@ -5510,6 +5534,9 @@ "save-library-panel": "Save library panel", "settings": "Dashboard settings", "share-button": "Share", + "snapshot": { + "title": "Source" + }, "star-add-error": "Failed to add to starred", "star-added": "Added to starred", "star-remove-error": "Failed to remove from starred", @@ -5839,9 +5866,6 @@ "name-values-separated-comma": "Values separated by comma", "selection-options": "Selection options" }, - "dashboard-edit-pane-renderer": { - "outline": "Outline" - }, "dashboard-link-form": { "back-to-list": "Back to list", "label-icon": "Icon", @@ -8121,13 +8145,6 @@ } } }, - "grafana": { - "dashboard": { - "edit-pane": { - "go-back": "Go back" - } - } - }, "grafana-data": { "datetime": { "rangeutils": { From d49993ddab08bf0d8daa544ec110ad710c7c27c0 Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Wed, 26 Nov 2025 15:45:36 +0200 Subject: [PATCH 05/11] Provisioning: Disable imports for new dashboard (#114419) * Provisioning: Disable imports for new dashboard * Refactor --- .../DashboardEmpty/DashboardEmpty.tsx | 10 +++++----- .../DashboardEmpty/DashboardEmptyHooks.ts | 20 +++++++++++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/DashboardEmpty/DashboardEmpty.tsx b/public/app/features/dashboard/dashgrid/DashboardEmpty/DashboardEmpty.tsx index bef4318e339..8c623c2c7ca 100644 --- a/public/app/features/dashboard/dashgrid/DashboardEmpty/DashboardEmpty.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardEmpty/DashboardEmpty.tsx @@ -15,7 +15,7 @@ import { SuggestedDashboards } from '../DashboardLibrary/SuggestedDashboards'; import { DashboardEmptyExtensionPoint } from './DashboardEmptyExtensionPoint'; import { - useIsReadOnlyRepo, + useRepositoryStatus, useOnAddVisualization, useOnAddLibraryPanel, useOnImportDashboard, @@ -149,10 +149,10 @@ export interface Props { // We pass the default empty UI through to the extension point so that the extension can conditionally render it if needed. // For example, an extension might want to render custom UI for a specific experiment cohort, and the default UI for everyone else. const DashboardEmpty = (props: Props) => { - const isReadOnlyRepo = useIsReadOnlyRepo(props); - const onAddVisualization = useOnAddVisualization({ ...props, isReadOnlyRepo }); - const onAddLibraryPanel = useOnAddLibraryPanel({ ...props, isReadOnlyRepo }); - const onImportDashboard = useOnImportDashboard({ ...props, isReadOnlyRepo }); + const { isReadOnlyRepo, isProvisioned } = useRepositoryStatus(props); + const onAddVisualization = useOnAddVisualization({ ...props, isReadOnlyRepo, isProvisioned }); + const onAddLibraryPanel = useOnAddLibraryPanel({ ...props, isReadOnlyRepo, isProvisioned }); + const onImportDashboard = useOnImportDashboard({ ...props, isReadOnlyRepo, isProvisioned }); return ( { - const { isReadOnlyRepo } = useGetResourceRepositoryView({ +export const useRepositoryStatus = ({ dashboard }: Props) => { + const { isReadOnlyRepo, repository } = useGetResourceRepositoryView({ folderName: dashboard instanceof DashboardScene ? dashboard.state.meta.folderUid : dashboard.meta.folderUid, }); - return isReadOnlyRepo; + const isFolderProvisioned = Boolean(repository); + const isProvisioned = isFolderProvisioned || (dashboard instanceof DashboardScene && dashboard.isManagedRepository()); + + return { + isReadOnlyRepo, + isProvisioned, + }; }; interface HookProps extends Props { isReadOnlyRepo: boolean; + isProvisioned: boolean; } export const useOnAddVisualization = ({ dashboard, canCreate, isReadOnlyRepo }: HookProps) => { @@ -53,9 +60,7 @@ export const useOnAddVisualization = ({ dashboard, canCreate, isReadOnlyRepo }: }, [canCreate, isReadOnlyRepo, dashboard, dispatch, initialDatasource]); }; -export const useOnAddLibraryPanel = ({ dashboard, canCreate, isReadOnlyRepo }: HookProps) => { - const isProvisioned = dashboard instanceof DashboardScene && dashboard.isManagedRepository(); - +export const useOnAddLibraryPanel = ({ dashboard, canCreate, isReadOnlyRepo, isProvisioned }: HookProps) => { return useMemo(() => { if (!canCreate || isProvisioned || isReadOnlyRepo) { return undefined; @@ -72,8 +77,7 @@ export const useOnAddLibraryPanel = ({ dashboard, canCreate, isReadOnlyRepo }: H }, [canCreate, isProvisioned, isReadOnlyRepo, dashboard]); }; -export const useOnImportDashboard = ({ dashboard, canCreate, isReadOnlyRepo }: HookProps) => { - const isProvisioned = dashboard instanceof DashboardScene && dashboard.isManagedRepository(); +export const useOnImportDashboard = ({ canCreate, isReadOnlyRepo, isProvisioned }: HookProps) => { return useMemo(() => { if (!canCreate || isProvisioned || isReadOnlyRepo) { return undefined; From 9606e9c51cb70d096df60d903e9c1de44514912f Mon Sep 17 00:00:00 2001 From: Jacob Valdez Date: Wed, 26 Nov 2025 07:48:18 -0600 Subject: [PATCH 06/11] Docs: Clarify the uid is metadata.name (#114432) --- .../developer-resources/api-reference/http-api/dashboard.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sources/developer-resources/api-reference/http-api/dashboard.md b/docs/sources/developer-resources/api-reference/http-api/dashboard.md index 5f28d4c54b1..2cc7aa84bb3 100644 --- a/docs/sources/developer-resources/api-reference/http-api/dashboard.md +++ b/docs/sources/developer-resources/api-reference/http-api/dashboard.md @@ -767,12 +767,12 @@ Status Codes: Deletes a dashboard via the dashboard uid. -- namespace: to read more about the namespace to use, see the [API overview](https://grafana.com/docs/grafana//developers/http_api/apis/). -- uid: the unique identifier of the dashboard to update. this will be the _name_ in the dashboard response +- **`namespace`**: To read more about the namespace to use, see the [API overview](https://grafana.com/docs/grafana//developers/http_api/apis/). +- **`uid`**: The unique identifier of the dashboard to update. This is the `metadata.name` field in the dashboard response and _not_ the `metadata.uid` field. **Required permissions** -See note in the [introduction]({{< ref "#dashboard-api" >}}) for an explanation. +See note in the [introduction](#new-dashboard-apis) for an explanation. | Action | Scope | From 84fbe6bc7ba6e9562d941551ff907797650e7dcd Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Wed, 26 Nov 2025 14:51:31 +0100 Subject: [PATCH 07/11] Alerting: Analyze an alert rule with Grafana Assistant (#114420) * fix * rename to analyze * Enable Analyze rule for GMA recording rules * Fix declare incident button condition --------- Co-authored-by: Konrad Lalik --- .../unified/PanelAlertTabContent.test.tsx | 3 + .../alerting/unified/RuleList.test.tsx | 3 + .../assistant/AnalizeRuleButton.tsx | 139 ++++++++++++++++++ .../components/rule-viewer/AlertRuleMenu.tsx | 6 + .../rule-viewer/RuleViewer.test.tsx | 4 + .../rules/RuleActionsButtons.test.tsx | 4 + .../rules/RuleListGroupView.test.tsx | 4 + .../rules/RuleListStateView.test.tsx | 4 + .../components/rules/RulesTable.test.tsx | 4 + .../group-details/GroupDetailsPage.test.tsx | 4 + .../rule-list/DataSourceGroupLoader.test.tsx | 4 + .../unified/rule-list/FilterView.test.tsx | 4 + .../rule-list/GrafanaGroupLoader.test.tsx | 76 ++++++++++ .../PanelDataAlertingTab.test.tsx | 9 +- public/locales/en-US/grafana.json | 1 + 15 files changed, 265 insertions(+), 4 deletions(-) create mode 100644 public/app/features/alerting/unified/components/assistant/AnalizeRuleButton.tsx diff --git a/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx b/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx index dfe3549eeee..dac15642d7b 100644 --- a/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx +++ b/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx @@ -28,6 +28,9 @@ import { Annotation } from './utils/constants'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; jest.mock('./api/ruler'); +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); jest.spyOn(alertingAbilities, 'useAlertRuleAbility'); const prometheusModuleSettings = { alerting: true, module: 'core:plugin/prometheus' }; diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index f58aadb9afd..3849c4708aa 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -45,6 +45,9 @@ jest.mock('@grafana/runtime', () => ({ jest.mock('./api/buildInfo'); jest.mock('./api/prometheus'); jest.mock('./api/ruler'); +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); jest.spyOn(actions, 'rulesInSameGroupHaveInvalidFor').mockReturnValue([]); jest.spyOn(apiRuler, 'rulerUrlBuilder'); diff --git a/public/app/features/alerting/unified/components/assistant/AnalizeRuleButton.tsx b/public/app/features/alerting/unified/components/assistant/AnalizeRuleButton.tsx new file mode 100644 index 00000000000..d3aa6db4ec0 --- /dev/null +++ b/public/app/features/alerting/unified/components/assistant/AnalizeRuleButton.tsx @@ -0,0 +1,139 @@ +import { useMemo } from 'react'; + +import { OpenAssistantProps, createAssistantContextItem, useAssistant } from '@grafana/assistant'; +import { t } from '@grafana/i18n'; +import { reportInteraction } from '@grafana/runtime'; +import { Menu } from '@grafana/ui'; +import { GrafanaAlertingRule, GrafanaRecordingRule, GrafanaRule } from 'app/types/unified-alerting'; + +import { prometheusRuleType } from '../../utils/rules'; + +interface AnalyzeRuleButtonProps { + /** Alert rule to analyze */ + rule: GrafanaRule; +} + +/** + * A menu item component that analyze an alert rule. + * Automatically creates context from alert data and opens the assistant in assistant mode. + */ +export function AnalyzeRuleButton(props: AnalyzeRuleButtonProps) { + const { isAvailable, openAssistant } = useAssistant(); + + if (!isAvailable || !openAssistant) { + return null; + } + + return ; +} + +function AnalyzeRuleButtonView({ + rule, + openAssistant, +}: AnalyzeRuleButtonProps & { + openAssistant: (props: OpenAssistantProps) => void; +}) { + // Create alert rule context from alert rule data + const alertContext = useMemo(() => { + return createAssistantContextItem('structured', { + title: `Alert: ${rule.name}`, + data: { + rule: { + name: rule.name, + uid: rule.uid, + labels: rule.labels, + query: rule.query, + }, + }, + }); + }, [rule]); + + // Generate default prompt + const analyzeRulePrompt = useMemo(() => buildAnalyzeRulePrompt(rule), [rule]); + + const handleClick = () => { + reportInteraction('grafana_assistant_app_analyze_rule_button_clicked', { + origin: 'alerting', + alertName: rule.name, + alertState: prometheusRuleType.grafana.alertingRule(rule) ? rule.state : undefined, + }); + + openAssistant({ + origin: 'alerting', + mode: 'assistant', + prompt: analyzeRulePrompt, + context: [alertContext], + autoSend: true, + }); + }; + + return ( + + ); +} + +/** + * Builds a prompt for analyzing a rule (alerting or recording). + * Automatically detects the rule type and uses the appropriate prompt builder. + */ +function buildAnalyzeRulePrompt(rule: GrafanaRule): string { + if (prometheusRuleType.grafana.alertingRule(rule)) { + return buildAnalyzeAlertingRulePrompt(rule); + } else if (prometheusRuleType.grafana.recordingRule(rule)) { + return buildAnalyzeRecordingRulePrompt(rule); + } + // Fallback (should not happen for GrafanaRule, but TypeScript requires it) + return `Analyze the rule "${rule.name}".`; +} + +/** + * Builds a prompt for analyzing an alerting rule. + * Includes state, activeAt timestamp, annotations, and labels. + */ +function buildAnalyzeAlertingRulePrompt(rule: GrafanaAlertingRule): string { + const state = rule.state || 'firing'; + const timeInfo = rule.activeAt ? ` starting at ${new Date(rule.activeAt).toISOString()}` : ''; + + let prompt = `Analyze the ${state} alert "${rule.name}"${timeInfo}.`; + + const description = rule.annotations?.description || rule.annotations?.summary || ''; + if (description) { + prompt += ` ${description}`; + } + + const labelsStr = rule.labels + ? Object.entries(rule.labels) + .map(([k, v]) => `${k}="${v}"`) + .join(', ') + : ''; + if (labelsStr) { + prompt += ` Labels: ${labelsStr}.`; + } + + return prompt; +} + +/** + * Builds a prompt for analyzing a recording rule. + * Includes name, query, and labels (no state or activeAt). + */ +function buildAnalyzeRecordingRulePrompt(rule: GrafanaRecordingRule): string { + const labelsStr = rule.labels + ? Object.entries(rule.labels) + .map(([k, v]) => `${k}="${v}"`) + .join(', ') + : ''; + + let prompt = `Analyze the recording rule "${rule.name}".`; + + if (labelsStr) { + prompt += ` Labels: ${labelsStr}.`; + } + + return prompt; +} diff --git a/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx b/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx index 8834fd72580..a473eae60a1 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx @@ -1,5 +1,6 @@ import { PropsOf } from '@emotion/react'; +import { useAssistant } from '@grafana/assistant'; import { AppEvents } from '@grafana/data'; import { t } from '@grafana/i18n'; import { config } from '@grafana/runtime'; @@ -29,6 +30,7 @@ import { rulerRuleType, } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; +import { AnalyzeRuleButton } from '../assistant/AnalizeRuleButton'; import { DeclareIncidentMenuItem } from '../bridges/DeclareIncidentButton'; interface Props { @@ -126,6 +128,9 @@ const AlertRuleMenu = ({ prometheusRuleType.alertingRule(promRule) && promRule.state === PromAlertingRuleState.Firing; + const { isAvailable: isAssistantAvailable } = useAssistant(); + const shouldShowAnalyzeRuleButton = isAssistantAvailable && prometheusRuleType.grafana.rule(promRule); + const shareUrl = createShareLink(identifier); const showDivider = @@ -172,6 +177,7 @@ const AlertRuleMenu = ({ )} {/* TODO Migrate Declare Incident to plugin links extensions */} {shouldShowDeclareIncidentButton && } + {shouldShowAnalyzeRuleButton && } {canDuplicate && ( ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + // metadata and interactive elements const ELEMENTS = { loading: byText(/Loading rule/i), diff --git a/public/app/features/alerting/unified/components/rules/RuleActionsButtons.test.tsx b/public/app/features/alerting/unified/components/rules/RuleActionsButtons.test.tsx index 5de673ccb14..a88d1bef83f 100644 --- a/public/app/features/alerting/unified/components/rules/RuleActionsButtons.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleActionsButtons.test.tsx @@ -21,6 +21,10 @@ import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import { setupDataSources } from '../../testSetup/datasources'; import { fromCombinedRule, stringifyIdentifier } from '../../utils/rule-id'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + setupMswServer(); jest.mock('app/core/services/context_srv'); const mockContextSrv = jest.mocked(contextSrv); diff --git a/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx b/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx index 19100a72600..42a7594027b 100644 --- a/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx @@ -15,6 +15,10 @@ import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { RuleListGroupView } from './RuleListGroupView'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + jest.spyOn(analytics, 'logInfo'); const ui = { diff --git a/public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx b/public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx index 260c7436a18..51739c6ae4c 100644 --- a/public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx @@ -10,6 +10,10 @@ import { } from 'app/features/alerting/unified/mocks'; import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + setPluginLinksHook(() => ({ links: [], isLoading: false, diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx index 61c143ab843..a263b4711ca 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx @@ -18,6 +18,10 @@ import { mimirDataSource } from '../../mocks/server/configure'; import { RulesTable } from './RulesTable'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + jest.mock('../../hooks/useAbilities'); const mocks = { diff --git a/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx b/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx index 90485290325..4e1bb3056fb 100644 --- a/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx +++ b/public/app/features/alerting/unified/group-details/GroupDetailsPage.test.tsx @@ -23,6 +23,10 @@ import { alertingFactory } from '../mocks/server/db'; import GroupDetailsPage from './GroupDetailsPage'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + jest.mock('react-virtualized-auto-sizer', () => { return ({ children }: Props) => children({ diff --git a/public/app/features/alerting/unified/rule-list/DataSourceGroupLoader.test.tsx b/public/app/features/alerting/unified/rule-list/DataSourceGroupLoader.test.tsx index 53b80f1ebaf..a057b5030a3 100644 --- a/public/app/features/alerting/unified/rule-list/DataSourceGroupLoader.test.tsx +++ b/public/app/features/alerting/unified/rule-list/DataSourceGroupLoader.test.tsx @@ -17,6 +17,10 @@ import { fromRulerRuleAndGroupIdentifierV2 } from '../utils/rule-id'; import { DataSourceGroupLoader } from './DataSourceGroupLoader'; import { createViewLinkFromIdentifier } from './DataSourceRuleListItem'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + setPluginLinksHook(() => ({ links: [], isLoading: false })); setPluginComponentsHook(() => ({ components: [], isLoading: false })); diff --git a/public/app/features/alerting/unified/rule-list/FilterView.test.tsx b/public/app/features/alerting/unified/rule-list/FilterView.test.tsx index 3a7348048da..5c8768f58ec 100644 --- a/public/app/features/alerting/unified/rule-list/FilterView.test.tsx +++ b/public/app/features/alerting/unified/rule-list/FilterView.test.tsx @@ -12,6 +12,10 @@ import { RulesFilter } from '../search/rulesSearchParser'; import { FilterView } from './FilterView'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + setPluginLinksHook(() => ({ links: [], isLoading: false })); setPluginComponentsHook(() => ({ components: [], isLoading: false })); diff --git a/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx b/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx index 3105c3efd6b..7385215637a 100644 --- a/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx +++ b/public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx @@ -1,6 +1,7 @@ import { render } from 'test/test-utils'; import { byLabelText, byRole } from 'testing-library-selector'; +import { useAssistant } from '@grafana/assistant'; import { setPluginComponentsHook, setPluginLinksHook } from '@grafana/runtime'; import { AccessControlAction } from 'app/types/accessControl'; import { GrafanaRuleGroupIdentifier } from 'app/types/unified-alerting'; @@ -22,6 +23,12 @@ import { intervalToSeconds } from '../utils/time'; import { GrafanaGroupLoader } from './GrafanaGroupLoader'; +jest.mock('@grafana/assistant', () => ({ + useAssistant: jest.fn(), + createAssistantContextItem: jest.fn((type, data) => ({ type, ...data })), +})); +const mockUseAssistant = jest.mocked(useAssistant); + setPluginLinksHook(() => ({ links: [], isLoading: false })); setPluginComponentsHook(() => ({ components: [], isLoading: false })); @@ -41,11 +48,18 @@ const ui = { export: () => byRole('menuitem', { name: /export/i }), delete: () => byRole('menuitem', { name: /delete/i }), pause: () => byRole('menuitem', { name: /pause/i }), + analyzeRule: () => byRole('menuitem', { name: /analyze rule/i }), }, }; describe('GrafanaGroupLoader', () => { beforeEach(() => { + mockUseAssistant.mockReturnValue({ + isAvailable: false, + openAssistant: jest.fn(), + closeAssistant: jest.fn(), + toggleAssistant: jest.fn(), + }); grantUserPermissions([ AccessControlAction.AlertingRuleUpdate, AccessControlAction.AlertingRuleDelete, @@ -213,6 +227,68 @@ describe('GrafanaGroupLoader', () => { const menuItems = byRole('menuitem').getAll(); expect(menuItems.length).toBe(6); }); + + it('should render Analyze rule menu item when assistant is available', async () => { + mockUseAssistant.mockReturnValue({ + isAvailable: true, + openAssistant: jest.fn(), + closeAssistant: jest.fn(), + toggleAssistant: jest.fn(), + }); + + setGrafanaPromRules([rulerGroupToPromGroup(grafanaRulerGroup)]); + + const groupIdentifier = getGroupIdentifier(grafanaRulerGroup); + + const { user } = render( + + ); + + const [rule1] = grafanaRulerGroup.rules; + const ruleListItem = await ui.ruleItem(rule1.grafana_alert.title).find(); + + // Click the More button to open the menu + const moreButton = ui.moreButton().get(ruleListItem); + await user.click(moreButton); + + // Check that Analyze rule menu item is present + expect(ui.menuItems.analyzeRule().get()).toBeInTheDocument(); + + // With assistant enabled, there should be 7 menu items (6 + Analyze rule) + const menuItems = byRole('menuitem').getAll(); + expect(menuItems.length).toBe(7); + }); + + it('should not render Analyze rule menu item when assistant is not available', async () => { + mockUseAssistant.mockReturnValue({ + isAvailable: false, + openAssistant: jest.fn(), + closeAssistant: jest.fn(), + toggleAssistant: jest.fn(), + }); + + setGrafanaPromRules([rulerGroupToPromGroup(grafanaRulerGroup)]); + + const groupIdentifier = getGroupIdentifier(grafanaRulerGroup); + + const { user } = render( + + ); + + const [rule1] = grafanaRulerGroup.rules; + const ruleListItem = await ui.ruleItem(rule1.grafana_alert.title).find(); + + // Click the More button to open the menu + const moreButton = ui.moreButton().get(ruleListItem); + await user.click(moreButton); + + // Check that Analyze rule menu item is NOT present + expect(ui.menuItems.analyzeRule().query()).not.toBeInTheDocument(); + + // Without assistant, there should be 6 menu items + const menuItems = byRole('menuitem').getAll(); + expect(menuItems.length).toBe(6); + }); }); function rulerGroupToPromGroup(group: RulerRuleGroupDTO): GrafanaPromRuleGroupDTO { diff --git a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataAlertingTab.test.tsx b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataAlertingTab.test.tsx index 15e323aa8a9..fd15ec40350 100644 --- a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataAlertingTab.test.tsx +++ b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataAlertingTab.test.tsx @@ -40,14 +40,15 @@ import { PanelDataAlertingTab, PanelDataAlertingTabRendered } from './PanelDataA jest.mock('app/features/alerting/unified/api/prometheus'); jest.mock('app/features/alerting/unified/api/ruler'); +jest.mock('@grafana/assistant', () => ({ + useAssistant: () => ({ isAvailable: false, openAssistant: jest.fn() }), +})); + jest.spyOn(ruleActionButtons, 'matchesWidth').mockReturnValue(false); jest.spyOn(ruler, 'rulerUrlBuilder'); jest.spyOn(alertingAbilities, 'useAlertRuleAbility'); -setPluginLinksHook(() => ({ - links: [], - isLoading: false, -})); +setPluginLinksHook(() => ({ links: [], isLoading: false })); const dataSources = { prometheus: mockDataSource( diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index dad607c0de0..9f8b19218de 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -477,6 +477,7 @@ "noOptionsMessage-no-datasources-found": "No datasources found" }, "alert-menu": { + "analyze-rule": "Analyze rule", "copy-link": "Copy link", "duplicate": "Duplicate", "export": "Export", From 759d49a1df0b30bde07289c0aecf7e042ba7914c Mon Sep 17 00:00:00 2001 From: Andres Torres Date: Wed, 26 Nov 2025 09:58:49 -0500 Subject: [PATCH 08/11] feat(setting): Adding setting service client (#114428) --- .github/CODEOWNERS | 1 + pkg/services/setting/service.go | 384 +++++++++++++++++++ pkg/services/setting/service_test.go | 542 +++++++++++++++++++++++++++ 3 files changed, 927 insertions(+) create mode 100644 pkg/services/setting/service.go create mode 100644 pkg/services/setting/service_test.go diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 61ab3a10b1f..465c49ee713 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -185,6 +185,7 @@ /pkg/services/search/ @grafana/grafana-search-and-storage /pkg/services/searchusers/ @grafana/grafana-search-and-storage /pkg/services/secrets/ @grafana/grafana-operator-experience-squad +/pkg/services/setting/ @grafana/grafana-backend-services-squad /pkg/services/shorturls/ @grafana/sharing-squad /pkg/services/sqlstore/ @grafana/grafana-search-and-storage /pkg/services/ssosettings/ @grafana/identity-squad diff --git a/pkg/services/setting/service.go b/pkg/services/setting/service.go new file mode 100644 index 00000000000..5afc9e22159 --- /dev/null +++ b/pkg/services/setting/service.go @@ -0,0 +1,384 @@ +package setting + +import ( + "context" + "fmt" + "net/http" + "time" + + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/prometheus/client_golang/prometheus" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/trace" + "gopkg.in/ini.v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + utilnet "k8s.io/apimachinery/pkg/util/net" + "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/client-go/dynamic" + clientrest "k8s.io/client-go/rest" + "k8s.io/client-go/transport" + + authlib "github.com/grafana/authlib/authn" + logging "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/semconv" +) + +var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/setting") + +const LogPrefix = "setting.service" + +const DefaultPageSize = int64(500) +const DefaultQPS = float32(10) +const DefaultBurst = 25 + +const ( + ApiGroup = "setting.grafana.app" + apiVersion = "v0alpha1" + resource = "settings" + kind = "Setting" + listKind = "SettingList" +) + +var settingGroupVersion = schema.GroupVersionResource{ + Group: ApiGroup, + Version: apiVersion, + Resource: resource, +} + +var settingGroupListKind = map[schema.GroupVersionResource]string{ + settingGroupVersion: listKind, +} + +type remoteSettingServiceMetrics struct { + listDuration *prometheus.HistogramVec + listResultSize *prometheus.HistogramVec +} + +// Service retrieves configuration settings from a remote settings service. +// +// The service uses label selectors to filter settings. Settings are labeled with +// "section" and "key" labels matching their spec fields. +// +// Example - Select all settings: +// +// ctx := request.WithNamespace(context.Background(), "my-namespace") +// ini, err := service.ListAsIni(ctx, metav1.LabelSelector{}) +// +// Example - Select settings from specific sections: +// +// selector := metav1.LabelSelector{ +// MatchExpressions: []metav1.LabelSelectorRequirement{ +// { +// Key: "section", +// Operator: metav1.LabelSelectorOpIn, +// Values: []string{"database", "server"}, +// }, +// }, +// } +// ini, err := service.ListAsIni(ctx, selector) +// +// Example - Select settings from a single section with specific labels: +// +// selector := metav1.LabelSelector{ +// MatchLabels: map[string]string{ +// "section": "database", +// }, +// } +// settings, err := service.List(ctx, selector) +type Service interface { + prometheus.Collector + // ListAsIni retrieves settings filtered by a label selector from the namespace in context + // and returns them as an ini.File. + // + // The namespace must be present in the context, ie: via request.WithNamespace. + // An empty selector returns all settings in the namespace. + ListAsIni(ctx context.Context, selector metav1.LabelSelector) (*ini.File, error) + + // List retrieves settings filtered by a label selector from the namespace in context + // and returns them as a slice of Setting structs. + // + // The namespace must be present in the context, ie: via request.WithNamespace. + // An empty selector returns all settings in the namespace. + List(ctx context.Context, selector metav1.LabelSelector) ([]*Setting, error) +} + +type remoteSettingService struct { + dynamicClient dynamic.Interface + log logging.Logger + pageSize int64 + metrics remoteSettingServiceMetrics +} + +var _ Service = (*remoteSettingService)(nil) +var _ prometheus.Collector = (*remoteSettingService)(nil) + +// Config configures a Service. +type Config struct { + // URL is the base URL for the remote settings service (required). + URL string + // TokenExchangeClient authenticates requests (required if WrapTransport is not set). + TokenExchangeClient *authlib.TokenExchangeClient + // WrapTransport wraps the HTTP transport for authentication. + // Takes precedence over TokenExchangeClient when both are set. + // At least one of WrapTransport or TokenExchangeClient is required. + WrapTransport transport.WrapperFunc + // TLSClientConfig configures TLS for the client connection. + TLSClientConfig clientrest.TLSClientConfig + // QPS limits requests per second (defaults to DefaultQPS). + QPS float32 + // Burst allows request bursts above QPS (defaults to DefaultBurst). + Burst int + // PageSize sets the number of items per API page (defaults to DefaultPageSize). + PageSize int64 +} + +// Setting represents the parsed spec of a Setting resource. +type Setting struct { + // Setting section + Section string `json:"section"` + // Setting key + Key string `json:"key"` + // Setting value + Value string `json:"value"` +} + +// New creates a Service from the provided configuration. +func New(config Config) (Service, error) { + log := logging.New(LogPrefix) + dynamicClient, err := getDynamicClient(config, log) + if err != nil { + return nil, err + } + pageSize := DefaultPageSize + if config.PageSize > 0 { + pageSize = config.PageSize + } + + metrics := initMetrics() + + return &remoteSettingService{ + dynamicClient: dynamicClient, + pageSize: pageSize, + log: log, + metrics: metrics, + }, nil +} + +func (m *remoteSettingService) ListAsIni(ctx context.Context, labelSelector metav1.LabelSelector) (*ini.File, error) { + namespace, ok := request.NamespaceFrom(ctx) + ns := semconv.GrafanaNamespaceName(namespace) + ctx, span := tracer.Start(ctx, "remoteSettingService.ListAsIni", + trace.WithAttributes(ns)) + defer span.End() + + if !ok || namespace == "" { + return nil, tracing.Errorf(span, "missing namespace in context") + } + + settings, err := m.List(ctx, labelSelector) + if err != nil { + return nil, err + } + iniFile, err := m.toIni(settings) + if err != nil { + return nil, tracing.Error(span, err) + } + return iniFile, nil +} + +func (m *remoteSettingService) List(ctx context.Context, labelSelector metav1.LabelSelector) ([]*Setting, error) { + namespace, ok := request.NamespaceFrom(ctx) + ns := semconv.GrafanaNamespaceName(namespace) + ctx, span := tracer.Start(ctx, "remoteSettingService.List", + trace.WithAttributes(ns)) + defer span.End() + if !ok || namespace == "" { + return nil, tracing.Errorf(span, "missing namespace in context") + } + log := m.log.FromContext(ctx).New(ns.Key, ns.Value, "function", "remoteSettingService.List", "traceId", span.SpanContext().TraceID()) + + startTime := time.Now() + var status string + defer func() { + duration := time.Since(startTime).Seconds() + m.metrics.listDuration.WithLabelValues(status).Observe(duration) + }() + + selector, err := metav1.LabelSelectorAsSelector(&labelSelector) + if err != nil { + status = "error" + return nil, tracing.Error(span, err) + } + if selector.Empty() { + log.Debug("empty selector. Fetching all settings") + } + + var allSettings []*Setting + var continueToken string + hasNext := true + totalPages := 0 + // Using an upper limit to prevent infinite loops + for hasNext && totalPages < 1000 { + totalPages++ + opts := metav1.ListOptions{ + Limit: m.pageSize, + Continue: continueToken, + } + if !selector.Empty() { + opts.LabelSelector = selector.String() + } + + settingsList, lErr := m.dynamicClient.Resource(settingGroupVersion).Namespace(namespace).List(ctx, opts) + if lErr != nil { + status = "error" + return nil, tracing.Error(span, lErr) + } + for i := range settingsList.Items { + setting, pErr := parseSettingResource(&settingsList.Items[i]) + if pErr != nil { + status = "error" + return nil, tracing.Error(span, pErr) + } + allSettings = append(allSettings, setting) + } + continueToken = settingsList.GetContinue() + if continueToken == "" { + hasNext = false + } + } + + status = "success" + m.metrics.listResultSize.WithLabelValues(status).Observe(float64(len(allSettings))) + + return allSettings, nil +} + +func parseSettingResource(setting *unstructured.Unstructured) (*Setting, error) { + spec, found, err := unstructured.NestedMap(setting.Object, "spec") + if err != nil { + return nil, fmt.Errorf("failed to get spec from setting: %w", err) + } + if !found { + return nil, fmt.Errorf("spec not found in setting %s", setting.GetName()) + } + + var result Setting + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(spec, &result); err != nil { + return nil, fmt.Errorf("failed to convert spec to Setting: %w", err) + } + + return &result, nil +} + +func (m *remoteSettingService) toIni(settings []*Setting) (*ini.File, error) { + conf := ini.Empty() + for _, setting := range settings { + if !conf.HasSection(setting.Section) { + _, _ = conf.NewSection(setting.Section) + } + _, err := conf.Section(setting.Section).NewKey(setting.Key, setting.Value) + if err != nil { + return nil, err + } + } + return conf, nil +} + +func getDynamicClient(config Config, log logging.Logger) (dynamic.Interface, error) { + if config.URL == "" { + return nil, fmt.Errorf("URL cannot be empty") + } + if config.WrapTransport == nil && config.TokenExchangeClient == nil { + return nil, fmt.Errorf("must set either TokenExchangeClient or WrapTransport") + } + + wrapTransport := config.WrapTransport + if config.WrapTransport == nil { + log.Debug("using default wrapTransport with TokenExchangeClient") + wrapTransport = func(rt http.RoundTripper) http.RoundTripper { + return &authRoundTripper{ + tokenClient: config.TokenExchangeClient, + transport: rt, + } + } + } + + qps := DefaultQPS + if config.QPS > 0 { + qps = config.QPS + } + + burst := DefaultBurst + if config.Burst > 0 { + burst = config.Burst + } + + return dynamic.NewForConfig(&clientrest.Config{ + Host: config.URL, + WrapTransport: wrapTransport, + TLSClientConfig: config.TLSClientConfig, + QPS: qps, + Burst: burst, + }) +} + +// authRoundTripper wraps an HTTP transport with token-based authentication. +type authRoundTripper struct { + tokenClient *authlib.TokenExchangeClient + transport http.RoundTripper +} + +var _ http.RoundTripper = (*authRoundTripper)(nil) + +func (a *authRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + token, err := a.tokenClient.Exchange(req.Context(), authlib.TokenExchangeRequest{ + Audiences: []string{ApiGroup}, + Namespace: "*", + }) + if err != nil { + return nil, fmt.Errorf("failed to exchange token: %w", err) + } + req = utilnet.CloneRequest(req) + + req.Header.Set("X-Access-Token", fmt.Sprintf("Bearer %s", token.Token)) + return a.transport.RoundTrip(req) +} + +func initMetrics() remoteSettingServiceMetrics { + metrics := remoteSettingServiceMetrics{ + listDuration: prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: "settings", + Subsystem: "service", + Name: "list_settings_duration_seconds", + Help: "Duration of remote settings service List operations", + NativeHistogramBucketFactor: 1.1, + }, + []string{"status"}, // status: "success" or "error" + ), + listResultSize: prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: "settings", + Subsystem: "service", + Name: "list_settings_result_size", + Help: "Number of settings returned by remote settings service List operations", + NativeHistogramBucketFactor: 1.1, + }, + []string{"status"}, // status: "success" or "error" + ), + } + return metrics +} + +func (m *remoteSettingService) Describe(descs chan<- *prometheus.Desc) { + m.metrics.listDuration.Describe(descs) + m.metrics.listResultSize.Describe(descs) +} + +func (m *remoteSettingService) Collect(metrics chan<- prometheus.Metric) { + m.metrics.listDuration.Collect(metrics) + m.metrics.listResultSize.Collect(metrics) +} diff --git a/pkg/services/setting/service_test.go b/pkg/services/setting/service_test.go new file mode 100644 index 00000000000..949ccc73a86 --- /dev/null +++ b/pkg/services/setting/service_test.go @@ -0,0 +1,542 @@ +package setting + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/client-go/dynamic/fake" + k8testing "k8s.io/client-go/testing" + + authlib "github.com/grafana/authlib/authn" + "github.com/grafana/grafana/pkg/infra/log" +) + +func TestRemoteSettingService_ListAsIni(t *testing.T) { + t.Run("should filter settings by label selector", func(t *testing.T) { + // Create multiple settings, only some matching the selector + setting1 := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "type", Value: "postgres"}) + setting2 := newUnstructuredSetting("test-namespace", Setting{Section: "server", Key: "port", Value: "3000"}) + setting3 := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "host", Value: "localhost"}) + + client := newTestClient(500, setting1, setting2, setting3) + + // Create a selector that should match only database settings + selector := metav1.LabelSelector{ + MatchLabels: map[string]string{ + "section": "database", + }, + } + + ctx := request.WithNamespace(context.Background(), "test-namespace") + result, err := client.ListAsIni(ctx, selector) + + require.NoError(t, err) + assert.NotNil(t, result) + // Should only have database settings, not server settings + assert.True(t, result.HasSection("database")) + assert.Equal(t, "postgres", result.Section("database").Key("type").String()) + assert.Equal(t, "localhost", result.Section("database").Key("host").String()) + // Should NOT have server settings + assert.False(t, result.HasSection("server")) + }) + + t.Run("should return all settings with empty selector", func(t *testing.T) { + // Create multiple settings across different sections + setting1 := newUnstructuredSetting("test-namespace", Setting{Section: "server", Key: "port", Value: "3000"}) + setting2 := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "type", Value: "mysql"}) + + client := newTestClient(500, setting1, setting2) + + // Empty selector should select everything + selector := metav1.LabelSelector{} + + ctx := request.WithNamespace(context.Background(), "test-namespace") + result, err := client.ListAsIni(ctx, selector) + + require.NoError(t, err) + assert.NotNil(t, result) + // Should have all settings from all sections + assert.True(t, result.HasSection("server")) + assert.Equal(t, "3000", result.Section("server").Key("port").String()) + assert.True(t, result.HasSection("database")) + assert.Equal(t, "mysql", result.Section("database").Key("type").String()) + }) +} + +func TestRemoteSettingService_List(t *testing.T) { + t.Run("should handle single page response", func(t *testing.T) { + setting := newUnstructuredSetting("test-namespace", Setting{Section: "server", Key: "port", Value: "3000"}) + + client := newTestClient(500, setting) + + ctx := request.WithNamespace(context.Background(), "test-namespace") + result, err := client.List(ctx, metav1.LabelSelector{}) + + require.NoError(t, err) + assert.Len(t, result, 1) + + spec := result[0] + assert.Equal(t, "server", spec.Section) + assert.Equal(t, "port", spec.Key) + assert.Equal(t, "3000", spec.Value) + }) + + t.Run("should handle multiple pages", func(t *testing.T) { + totalPages := 3 + pageSize := 5 + + pages := make([][]*unstructured.Unstructured, totalPages) + for pageNum := 0; pageNum < totalPages; pageNum++ { + for idx := 0; idx < pageSize; idx++ { + item := newUnstructuredSetting( + "test-namespace", + Setting{ + Section: fmt.Sprintf("section-%d", pageNum), + Key: fmt.Sprintf("key-%d", idx), + Value: fmt.Sprintf("val-%d-%d", pageNum, idx), + }, + ) + pages[pageNum] = append(pages[pageNum], item) + } + } + + scheme := runtime.NewScheme() + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind) + listCallCount := 0 + dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) { + listCallCount++ + + continueToken := fmt.Sprintf("continue-%d", listCallCount) + if listCallCount == totalPages { + continueToken = "" + } + + if listCallCount <= totalPages { + list := &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "apiVersion": ApiGroup + "/" + apiVersion, + "kind": listKind, + }, + } + list.SetContinue(continueToken) + for _, item := range pages[listCallCount-1] { + list.Items = append(list.Items, *item) + } + return true, list, nil + } + + return false, nil, nil + }) + + client := &remoteSettingService{ + dynamicClient: dynamicClient, + pageSize: int64(pageSize), + log: log.NewNopLogger(), + metrics: initMetrics(), + } + + ctx := request.WithNamespace(context.Background(), "test-namespace") + result, err := client.List(ctx, metav1.LabelSelector{}) + + require.NoError(t, err) + assert.Len(t, result, totalPages*pageSize) + assert.Equal(t, totalPages, listCallCount) + }) + + t.Run("should pass label selector when provided", func(t *testing.T) { + scheme := runtime.NewScheme() + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind) + dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) { + listAction := action.(k8testing.ListActionImpl) + assert.Equal(t, "app=grafana", listAction.ListOptions.LabelSelector) + return true, &unstructured.UnstructuredList{}, nil + }) + + client := &remoteSettingService{ + dynamicClient: dynamicClient, + pageSize: 500, + log: log.NewNopLogger(), + metrics: initMetrics(), + } + + ctx := request.WithNamespace(context.Background(), "test-namespace") + _, err := client.List(ctx, metav1.LabelSelector{MatchLabels: map[string]string{"app": "grafana"}}) + + require.NoError(t, err) + }) + + t.Run("should stop pagination at 1000 pages", func(t *testing.T) { + scheme := runtime.NewScheme() + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind) + listCallCount := 0 + dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) { + listCallCount++ + // Always return a continue token to simulate infinite pagination + list := &unstructured.UnstructuredList{} + list.SetContinue("continue-forever") + return true, list, nil + }) + + client := &remoteSettingService{ + dynamicClient: dynamicClient, + pageSize: 10, + log: log.NewNopLogger(), + metrics: initMetrics(), + } + + ctx := request.WithNamespace(context.Background(), "test-namespace") + _, err := client.List(ctx, metav1.LabelSelector{}) + + require.NoError(t, err) + assert.Equal(t, 1000, listCallCount, "Should stop at 1000 pages to prevent infinite loops") + }) + + t.Run("should return error when parsing setting fails", func(t *testing.T) { + scheme := runtime.NewScheme() + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind) + dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) { + // Return a malformed setting without spec + list := &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "apiVersion": ApiGroup + "/" + apiVersion, + "kind": listKind, + }, + } + malformedSetting := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": ApiGroup + "/" + apiVersion, + "kind": kind, + "metadata": map[string]interface{}{ + "name": "malformed", + "namespace": "test-namespace", + }, + // Missing spec + }, + } + list.Items = append(list.Items, *malformedSetting) + return true, list, nil + }) + + client := &remoteSettingService{ + dynamicClient: dynamicClient, + pageSize: 500, + log: log.NewNopLogger(), + metrics: initMetrics(), + } + + ctx := request.WithNamespace(context.Background(), "test-namespace") + result, err := client.List(ctx, metav1.LabelSelector{}) + + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "spec not found") + }) +} + +func TestParseSettingResource(t *testing.T) { + t.Run("should parse valid setting resource", func(t *testing.T) { + setting := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "type", Value: "postgres"}) + + result, err := parseSettingResource(setting) + + require.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "database", result.Section) + assert.Equal(t, "type", result.Key) + assert.Equal(t, "postgres", result.Value) + }) + + t.Run("should return error when spec is missing", func(t *testing.T) { + setting := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": ApiGroup + "/" + apiVersion, + "kind": kind, + "metadata": map[string]interface{}{ + "name": "test-setting", + "namespace": "test-namespace", + }, + // No spec + }, + } + + result, err := parseSettingResource(setting) + + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "spec not found") + }) +} + +func TestRemoteSettingService_ToIni(t *testing.T) { + t.Run("should convert settings to ini format", func(t *testing.T) { + settings := []*Setting{ + {Section: "database", Key: "type", Value: "postgres"}, + {Section: "database", Key: "host", Value: "localhost"}, + {Section: "server", Key: "http_port", Value: "3000"}, + } + + client := &remoteSettingService{ + pageSize: 500, + log: log.NewNopLogger(), + } + + result, err := client.toIni(settings) + + require.NoError(t, err) + assert.NotNil(t, result) + assert.True(t, result.HasSection("database")) + assert.True(t, result.HasSection("server")) + assert.Equal(t, "postgres", result.Section("database").Key("type").String()) + assert.Equal(t, "localhost", result.Section("database").Key("host").String()) + assert.Equal(t, "3000", result.Section("server").Key("http_port").String()) + }) + + t.Run("should handle empty settings list", func(t *testing.T) { + var settings []*Setting + + client := &remoteSettingService{ + pageSize: 500, + log: log.NewNopLogger(), + } + + result, err := client.toIni(settings) + + require.NoError(t, err) + assert.NotNil(t, result) + sections := result.Sections() + assert.Len(t, sections, 1) // Only default section + }) + + t.Run("should create section if it does not exist", func(t *testing.T) { + settings := []*Setting{ + {Section: "new_section", Key: "new_key", Value: "new_value"}, + } + + client := &remoteSettingService{ + pageSize: 500, + log: log.NewNopLogger(), + } + + result, err := client.toIni(settings) + + require.NoError(t, err) + assert.True(t, result.HasSection("new_section")) + assert.Equal(t, "new_value", result.Section("new_section").Key("new_key").String()) + }) + + t.Run("should handle multiple keys in same section", func(t *testing.T) { + settings := []*Setting{ + {Section: "auth", Key: "disable_login_form", Value: "false"}, + {Section: "auth", Key: "disable_signout_menu", Value: "true"}, + } + + client := &remoteSettingService{ + pageSize: 500, + log: log.NewNopLogger(), + } + + result, err := client.toIni(settings) + + require.NoError(t, err) + assert.True(t, result.HasSection("auth")) + authSection := result.Section("auth") + assert.Equal(t, "false", authSection.Key("disable_login_form").String()) + assert.Equal(t, "true", authSection.Key("disable_signout_menu").String()) + }) +} + +func TestNew(t *testing.T) { + t.Run("should create client with default page size", func(t *testing.T) { + config := Config{ + URL: "https://example.com", + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + } + + client, err := New(config) + + require.NoError(t, err) + assert.NotNil(t, client) + remoteClient := client.(*remoteSettingService) + assert.Equal(t, DefaultPageSize, remoteClient.pageSize) + }) + + t.Run("should create client with custom page size", func(t *testing.T) { + config := Config{ + URL: "https://example.com", + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + PageSize: 100, + } + + client, err := New(config) + + require.NoError(t, err) + assert.NotNil(t, client) + remoteClient := client.(*remoteSettingService) + assert.Equal(t, int64(100), remoteClient.pageSize) + }) + + t.Run("should use default page size when zero is provided", func(t *testing.T) { + config := Config{ + URL: "https://example.com", + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + PageSize: 0, + } + + client, err := New(config) + + require.NoError(t, err) + assert.NotNil(t, client) + remoteClient := client.(*remoteSettingService) + assert.Equal(t, DefaultPageSize, remoteClient.pageSize) + }) + + t.Run("should return error when config is invalid", func(t *testing.T) { + config := Config{ + URL: "", // Invalid: empty URL + } + + client, err := New(config) + + require.Error(t, err) + assert.Nil(t, client) + assert.Contains(t, err.Error(), "URL cannot be empty") + }) +} + +func TestGetDynamicClient(t *testing.T) { + logger := log.NewNopLogger() + + t.Run("should return error when SettingServiceURL is empty", func(t *testing.T) { + config := Config{ + URL: "", + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + } + + client, err := getDynamicClient(config, logger) + + require.Error(t, err) + assert.Nil(t, client) + assert.Contains(t, err.Error(), "URL cannot be empty") + }) + + t.Run("should return error when both TokenExchangeClient and WrapTransport are nil", func(t *testing.T) { + config := Config{ + URL: "https://example.com", + TokenExchangeClient: nil, + WrapTransport: nil, + } + + client, err := getDynamicClient(config, logger) + + require.Error(t, err) + assert.Nil(t, client) + assert.Contains(t, err.Error(), "must set either TokenExchangeClient or WrapTransport") + }) + + t.Run("should create client with WrapTransport", func(t *testing.T) { + config := Config{ + URL: "https://example.com", + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + } + + client, err := getDynamicClient(config, logger) + + require.NoError(t, err) + assert.NotNil(t, client) + }) + + t.Run("should not fail when QPS and Burst are not provided", func(t *testing.T) { + config := Config{ + URL: "https://example.com", + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + } + + client, err := getDynamicClient(config, logger) + + require.NoError(t, err) + assert.NotNil(t, client) + }) + + t.Run("should not fail when custom QPS and Burst are provided", func(t *testing.T) { + config := Config{ + URL: "https://example.com", + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + QPS: 10.0, + Burst: 20, + } + + client, err := getDynamicClient(config, logger) + + require.NoError(t, err) + assert.NotNil(t, client) + }) + + t.Run("should use WrapTransport when both WrapTransport and TokenExchangeClient are provided", func(t *testing.T) { + wrapTransportCalled := false + tokenExchangeClient := &authlib.TokenExchangeClient{} + + config := Config{ + URL: "https://example.com", + TokenExchangeClient: tokenExchangeClient, + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { + wrapTransportCalled = true + return rt + }, + } + + client, err := getDynamicClient(config, logger) + + require.NoError(t, err) + assert.NotNil(t, client) + assert.True(t, wrapTransportCalled, "WrapTransport should be called and take precedence over TokenExchangeClient") + }) +} + +// Helper function to create an unstructured Setting object for tests +func newUnstructuredSetting(namespace string, spec Setting) *unstructured.Unstructured { + // Generate resource name in the format {section}--{key} + name := fmt.Sprintf("%s--%s", spec.Section, spec.Key) + + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": ApiGroup + "/" + apiVersion, + "kind": kind, + "metadata": map[string]interface{}{ + "name": name, + "namespace": namespace, + }, + "spec": map[string]interface{}{ + "section": spec.Section, + "key": spec.Key, + "value": spec.Value, + }, + }, + } + // Always set section and key labels + obj.SetLabels(map[string]string{ + "section": spec.Section, + "key": spec.Key, + }) + return obj +} + +// Helper function to create a test client with the dynamic fake client +func newTestClient(pageSize int64, objects ...runtime.Object) *remoteSettingService { + scheme := runtime.NewScheme() + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind, objects...) + + return &remoteSettingService{ + dynamicClient: dynamicClient, + pageSize: pageSize, + log: log.NewNopLogger(), + metrics: initMetrics(), + } +} From 4130bd9cd300ec7ce0fb492b3a7229968c9167b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mustafa=20Sencer=20=C3=96zcan?= <32759850+mustafasencer@users.noreply.github.com> Date: Wed, 26 Nov 2025 16:15:24 +0100 Subject: [PATCH 09/11] Revert "K8s: read resource configs from API Enablement for API Builders" (#114475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert "K8s: read resource configs from API Enablement for API Builders (#114…" This reverts commit 0c2707bbc41b60124d2217547f607e08a2a3f013. --- pkg/registry/apis/ofrep/register.go | 4 + pkg/services/apiserver/builder/helper.go | 106 +++++------------- pkg/services/apiserver/builder/openapi.go | 19 +--- .../apiserver/builder/request_handler.go | 10 +- pkg/services/apiserver/service.go | 8 +- 5 files changed, 36 insertions(+), 111 deletions(-) diff --git a/pkg/registry/apis/ofrep/register.go b/pkg/registry/apis/ofrep/register.go index d6a443cfee7..cd6f8f0eb29 100644 --- a/pkg/registry/apis/ofrep/register.go +++ b/pkg/registry/apis/ofrep/register.go @@ -64,6 +64,10 @@ func NewAPIBuilder(providerType string, url *url.URL, insecure bool, caFile stri } func RegisterAPIService(apiregistration builder.APIRegistrar, cfg *setting.Cfg) (*APIBuilder, error) { + if !cfg.OpenFeature.APIEnabled { + return nil, nil + } + var staticEvaluator featuremgmt.StaticFlagEvaluator // No static evaluator needed for non-static provider var err error if cfg.OpenFeature.ProviderType == setting.StaticProviderType { diff --git a/pkg/services/apiserver/builder/helper.go b/pkg/services/apiserver/builder/helper.go index ae90295c01d..603c49f3cb2 100644 --- a/pkg/services/apiserver/builder/helper.go +++ b/pkg/services/apiserver/builder/helper.go @@ -22,7 +22,6 @@ import ( k8srequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" genericapiserver "k8s.io/apiserver/pkg/server" - serverstorage "k8s.io/apiserver/pkg/server/storage" "k8s.io/apiserver/pkg/util/openapi" k8sscheme "k8s.io/client-go/kubernetes/scheme" k8stracing "k8s.io/component-base/tracing" @@ -73,36 +72,13 @@ var PathRewriters = []filters.PathRewriter{ }, } -// GetDefaultBuildHandlerChainFuncForAggregator is a replica of GetDefaultBuildHandlerChainFunc except it skips custom routes handling -func GetDefaultBuildHandlerChainFuncForAggregator() BuildHandlerChainFunc { - return func(delegateHandler http.Handler, c *genericapiserver.Config) http.Handler { - // filters.WithRequester needs to be after the K8s chain because it depends on the K8s user in context - handler := filters.WithRequester(delegateHandler) - - // Call DefaultBuildHandlerChain on the main entrypoint http.Handler - // See https://github.com/kubernetes/apiserver/blob/v0.28.0/pkg/server/config.go#L906 - // DefaultBuildHandlerChain provides many things, notably CORS, HSTS, cache-control, authz and latency tracking - handler = genericapiserver.DefaultBuildHandlerChain(handler, c) - - handler = filters.WithAcceptHeader(handler) - handler = filters.WithPathRewriters(handler, PathRewriters) - handler = k8stracing.WithTracing(handler, c.TracerProvider, "KubernetesAPI") - handler = filters.WithExtractJaegerTrace(handler) - // Configure filters.WithPanicRecovery to not crash on panic - utilruntime.ReallyCrash = false - - return handler - } -} func GetDefaultBuildHandlerChainFunc(builders []APIGroupBuilder, reg prometheus.Registerer) BuildHandlerChainFunc { return func(delegateHandler http.Handler, c *genericapiserver.Config) http.Handler { requestHandler, err := GetCustomRoutesHandler( delegateHandler, c.LoopbackClientConfig, builders, - reg, - c.MergedResourceConfig, - ) + reg) if err != nil { panic(fmt.Sprintf("could not build the request handler for specified API builders: %s", err.Error())) } @@ -129,8 +105,6 @@ func GetDefaultBuildHandlerChainFunc(builders []APIGroupBuilder, reg prometheus. } } -// SetupConfig sets up the server config for the API server -// specify isAggregator=true, if the chain is being constructed for kube-aggregator func SetupConfig( scheme *runtime.Scheme, serverConfig *genericapiserver.RecommendedConfig, @@ -140,7 +114,6 @@ func SetupConfig( gvs []schema.GroupVersion, additionalOpenAPIDefGetters []common.GetOpenAPIDefinitions, reg prometheus.Registerer, - apiResourceConfig *serverstorage.ResourceConfig, ) error { serverConfig.AdmissionControl = NewAdmissionFromBuilders(builders) defsGetter := GetOpenAPIDefinitions(builders, additionalOpenAPIDefGetters...) @@ -153,7 +126,7 @@ func SetupConfig( openapinamer.NewDefinitionNamer(scheme, k8sscheme.Scheme)) // Add the custom routes to service discovery - serverConfig.OpenAPIV3Config.PostProcessSpec = getOpenAPIPostProcessor(buildVersion, builders, gvs, apiResourceConfig) + serverConfig.OpenAPIV3Config.PostProcessSpec = getOpenAPIPostProcessor(buildVersion, builders, gvs) serverConfig.OpenAPIV3Config.GetOperationIDAndTagsFromRoute = func(r common.Route) (string, []string, error) { meta := r.Metadata() kind := "" @@ -314,7 +287,6 @@ func InstallAPIs( features featuremgmt.FeatureToggles, dualWriterMetrics *grafanarest.DualWriterMetrics, builderMetrics *BuilderMetrics, - apiResourceConfig *serverstorage.ResourceConfig, ) error { // dual writing is only enabled when the storage type is not legacy. // this is needed to support setting a default RESTOptionsGetter for new APIs that don't @@ -429,9 +401,34 @@ func InstallAPIs( for group, buildersForGroup := range buildersGroupMap { g := genericapiserver.NewDefaultAPIGroupInfo(group, scheme, metav1.ParameterCodec, codecs) for _, b := range buildersForGroup { - if err := installAPIGroupsForBuilder(&g, group, b, apiResourceConfig, scheme, optsGetter, dualWrite, reg, optsregister, storageOpts, features); err != nil { + if err := b.UpdateAPIGroupInfo(&g, APIGroupOptions{ + Scheme: scheme, + OptsGetter: optsGetter, + DualWriteBuilder: dualWrite, + MetricsRegister: reg, + StorageOptsRegister: optsregister, + StorageOpts: storageOpts, + }); err != nil { return err } + if len(g.PrioritizedVersions) < 1 { + continue + } + + // if grafanaAPIServerWithExperimentalAPIs is not enabled, remove v0alpha1 resources unless explicitly allowed + //nolint:staticcheck // not yet migrated to OpenFeature + if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) { + if resources, ok := g.VersionedResourcesStorageMap["v0alpha1"]; ok { + for name := range resources { + if !allowRegisteringResourceByInfo(b.AllowedV0Alpha1Resources(), name) { + delete(resources, name) + } + } + if len(resources) == 0 { + delete(g.VersionedResourcesStorageMap, "v0alpha1") + } + } + } } // skip installing the group if there are no resources left after filtering @@ -448,53 +445,6 @@ func InstallAPIs( return nil } -func installAPIGroupsForBuilder(g *genericapiserver.APIGroupInfo, group string, b APIGroupBuilder, apiResourceConfig *serverstorage.ResourceConfig, scheme *runtime.Scheme, - optsGetter generic.RESTOptionsGetter, dualWrite grafanarest.DualWriteBuilder, reg prometheus.Registerer, optsregister apistore.StorageOptionsRegister, - storageOpts *options.StorageOptions, features featuremgmt.FeatureToggles) error { - if err := b.UpdateAPIGroupInfo(g, APIGroupOptions{ - Scheme: scheme, - OptsGetter: optsGetter, - DualWriteBuilder: dualWrite, - MetricsRegister: reg, - StorageOptsRegister: optsregister, - StorageOpts: storageOpts, - }); err != nil { - return err - } - if len(g.PrioritizedVersions) < 1 { - return nil - } - - // filter out api groups that are disabled in APIEnablementOptions - for version := range g.VersionedResourcesStorageMap { - gvr := schema.GroupVersionResource{ - Group: group, - Version: version, - } - if apiResourceConfig != nil && !apiResourceConfig.ResourceEnabled(gvr) { - klog.InfoS("Skipping storage for disabled resource", "gvr", gvr.String()) - delete(g.VersionedResourcesStorageMap, version) - } - } - - // if grafanaAPIServerWithExperimentalAPIs is not enabled, remove v0alpha1 resources unless explicitly allowed - //nolint:staticcheck // not yet migrated to OpenFeature - if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) { - if resources, ok := g.VersionedResourcesStorageMap["v0alpha1"]; ok { - for name := range resources { - if !allowRegisteringResourceByInfo(b.AllowedV0Alpha1Resources(), name) { - delete(resources, name) - } - } - if len(resources) == 0 { - delete(g.VersionedResourcesStorageMap, "v0alpha1") - } - } - } - - return nil -} - // AddPostStartHooks adds post start hooks to a generic API server config func AddPostStartHooks( config *genericapiserver.RecommendedConfig, diff --git a/pkg/services/apiserver/builder/openapi.go b/pkg/services/apiserver/builder/openapi.go index 6d3b33f4baf..9cb7cc17f7c 100644 --- a/pkg/services/apiserver/builder/openapi.go +++ b/pkg/services/apiserver/builder/openapi.go @@ -9,8 +9,6 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime/schema" - serverstorage "k8s.io/apiserver/pkg/server/storage" - "k8s.io/klog/v2" openapi "k8s.io/kube-openapi/pkg/common" "k8s.io/kube-openapi/pkg/spec3" spec "k8s.io/kube-openapi/pkg/validation/spec" @@ -78,7 +76,6 @@ func addBuilderRoutes( targetGroupVersion schema.GroupVersion, openAPISpec *spec3.OpenAPI, apiGroupBuilders []APIGroupBuilder, - apiResourceConfig *serverstorage.ResourceConfig, ) (*spec3.OpenAPI, error) { for _, apiGroupBuilder := range apiGroupBuilders { // Optionally include raw http handlers for all builders @@ -110,24 +107,12 @@ func addBuilderRoutes( } } } - - // filter out api groups that are disabled in APIEnablementOptions - for path := range openAPISpec.Paths.Paths { - if strings.HasPrefix(path, "/apis/"+targetGroupVersion.String()+"/") { - gv := targetGroupVersion.WithResource("") - if apiResourceConfig != nil && !apiResourceConfig.ResourceEnabled(gv) { - klog.InfoS("removing openapi routes for disabled resource", "gv", gv.String()) - delete(openAPISpec.Paths.Paths, path) - } - } - } - return openAPISpec, nil } // Modify the OpenAPI spec to include the additional routes. // nolint:gocyclo -func getOpenAPIPostProcessor(version string, builders []APIGroupBuilder, gvs []schema.GroupVersion, apiResourceConfig *serverstorage.ResourceConfig) func(*spec3.OpenAPI) (*spec3.OpenAPI, error) { +func getOpenAPIPostProcessor(version string, builders []APIGroupBuilder, gvs []schema.GroupVersion) func(*spec3.OpenAPI) (*spec3.OpenAPI, error) { return func(s *spec3.OpenAPI) (*spec3.OpenAPI, error) { if s.Paths == nil { return s, nil @@ -242,7 +227,7 @@ func getOpenAPIPostProcessor(version string, builders []APIGroupBuilder, gvs []s } } } - return addBuilderRoutes(gv, ©, builders, apiResourceConfig) + return addBuilderRoutes(gv, ©, builders) } } return s, nil diff --git a/pkg/services/apiserver/builder/request_handler.go b/pkg/services/apiserver/builder/request_handler.go index 50761f1d42c..80fbf6d0eaa 100644 --- a/pkg/services/apiserver/builder/request_handler.go +++ b/pkg/services/apiserver/builder/request_handler.go @@ -6,9 +6,7 @@ import ( "github.com/gorilla/mux" "github.com/prometheus/client_golang/prometheus" - serverstorage "k8s.io/apiserver/pkg/server/storage" restclient "k8s.io/client-go/rest" - klog "k8s.io/klog/v2" "k8s.io/kube-openapi/pkg/spec3" ) @@ -16,7 +14,7 @@ type requestHandler struct { router *mux.Router } -func GetCustomRoutesHandler(delegateHandler http.Handler, restConfig *restclient.Config, builders []APIGroupBuilder, metricsRegistry prometheus.Registerer, apiResourceConfig *serverstorage.ResourceConfig) (http.Handler, error) { +func GetCustomRoutesHandler(delegateHandler http.Handler, restConfig *restclient.Config, builders []APIGroupBuilder, metricsRegistry prometheus.Registerer) (http.Handler, error) { useful := false // only true if any routes exist anywhere router := mux.NewRouter() @@ -29,12 +27,6 @@ func GetCustomRoutesHandler(delegateHandler http.Handler, restConfig *restclient } for _, gv := range GetGroupVersions(builder) { - // filter out api groups that are disabled in APIEnablementOptions - gvr := gv.WithResource("") - if apiResourceConfig != nil && !apiResourceConfig.ResourceEnabled(gvr) { - klog.InfoS("Skipping custom route handler for disabled group version", "gv", gv.String()) - continue - } routes := provider.GetAPIRoutes(gv) if routes == nil { continue diff --git a/pkg/services/apiserver/service.go b/pkg/services/apiserver/service.go index 605016c112d..28da416536c 100644 --- a/pkg/services/apiserver/service.go +++ b/pkg/services/apiserver/service.go @@ -316,11 +316,7 @@ func (s *service) start(ctx context.Context) error { s.cfg.BuildBranch, ) - apiResourceConfig := appinstaller.NewAPIResourceConfig(s.appInstallers) - // add the builder group versions to the api resource config - apiResourceConfig.EnableVersions(groupVersions...) - - if err := o.APIEnablementOptions.ApplyTo(&serverConfig.Config, apiResourceConfig, s.scheme); err != nil { + if err := o.APIEnablementOptions.ApplyTo(&serverConfig.Config, appinstaller.NewAPIResourceConfig(s.appInstallers), s.scheme); err != nil { return err } @@ -363,7 +359,6 @@ func (s *service) start(ctx context.Context) error { groupVersions, defGetters, s.metrics, - apiResourceConfig, ) if err != nil { return err @@ -405,7 +400,6 @@ func (s *service) start(ctx context.Context) error { s.features, s.dualWriterMetrics, s.builderMetrics, - apiResourceConfig, ) if err != nil { return err From 51d562eb813503bcb4327728fe358d20daa8df45 Mon Sep 17 00:00:00 2001 From: Jacob Valdez Date: Wed, 26 Nov 2025 09:21:12 -0600 Subject: [PATCH 10/11] Docs: Clarify some language in migration assistant docs (#114440) Co-authored-by: Irene Rodriguez --- .../migration-guide/cloud-migration-assistant.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sources/administration/migration-guide/cloud-migration-assistant.md b/docs/sources/administration/migration-guide/cloud-migration-assistant.md index 439e04c282d..b4a1e5b89fb 100644 --- a/docs/sources/administration/migration-guide/cloud-migration-assistant.md +++ b/docs/sources/administration/migration-guide/cloud-migration-assistant.md @@ -14,10 +14,10 @@ weight: 400 The Grafana Cloud Migration Assistant, generally available from Grafana v12.0, automatically migrates resources from your Grafana OSS/Enterprise instance to Grafana Cloud. It provides the following functionality: - Securely connect your self-managed instance to a Grafana Cloud instance. -- Seamlessly migrate resources such as dashboards, data sources, and folders to your cloud instance in a few easy steps. +- Migrate resources such as dashboards, data sources, and folders to your cloud instance in a few easy steps. - View the migration status of your resources in real-time. -Some of the benefits of the migration assistant are: +Some benefits of the migration assistant are: Ease of use : Follow the steps provided by the UI to easily migrate all your resources to Grafana Cloud without using Grafana APIs or scripts. @@ -44,7 +44,7 @@ The following resources are supported by the migration assistant: To use the Grafana migration assistant, you need: -- Grafana v11.2 or above with the `onPremToCloudMigrations` feature toggle enabled. In Grafana 11.5, this is enabled by default. For more information on how to enable a feature toggle, refer to [Configure feature toggles](https://grafana.com/docs/grafana//setup-grafana/configure-grafana/feature-toggles/#configure-feature-toggles). +- A self-managed Grafana instance version v11.2 or above with the `onPremToCloudMigrations` feature toggle enabled. In Grafana 11.5, this is enabled by default. For more information on how to enable a feature toggle, refer to [Configure feature toggles](https://grafana.com/docs/grafana//setup-grafana/configure-grafana/feature-toggles/#configure-feature-toggles). - A [Grafana Cloud Stack](https://grafana.com/docs/grafana-cloud/get-started/) you intend to migrate your resources to. - [`Admin`](https://grafana.com/docs/grafana-cloud/account-management/authentication-and-permissions/cloud-roles/) access to the Grafana Cloud Stack. To check your access level, go to `https://grafana.com/orgs//members`. - [Grafana server administrator](https://grafana.com/docs/grafana//administration/roles-and-permissions/#grafana-server-administrators) access to your existing Grafana OSS/Enterprise instance. To check your access level, go to `https:///admin/users`. @@ -64,7 +64,7 @@ In Grafana Enterprise, the server administrator has access to the migration assi ### Grant access in Grafana Enterprise -{{< admonition type="important">}} +{{< admonition type="note" >}} You must [configure RBAC](https://grafana.com/docs/grafana//administration/roles-and-permissions/access-control/configure-rbac/) before you can grant other administrators access to the Grafana Migration Assistant. {{< /admonition >}} From ca8cad68c8764a8f6653900de21dc39024b6d40f Mon Sep 17 00:00:00 2001 From: beejeebus Date: Tue, 25 Nov 2025 16:47:04 +0000 Subject: [PATCH 11/11] Add a metric to track usage of datasource configuration CRUD This PR adds `ds_config_handler_requests_duration_seconds` metric to help us track the release of the new datasource configuration CRUD api. Fixes https://github.com/grafana/grafana-enterprise/issues/10309 --- pkg/api/datasources.go | 21 +++++++++ pkg/api/datasources_test.go | 23 +++++++++ pkg/api/http_server.go | 49 +++++++++++--------- pkg/registry/apis/datasource/legacy_store.go | 28 ++++++++++- pkg/registry/apis/datasource/register.go | 6 +++ 5 files changed, 104 insertions(+), 23 deletions(-) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index cba7f5bc202..d7dc89d248f 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -9,6 +9,7 @@ import ( "sort" "strconv" "strings" + "time" "github.com/grafana/grafana-plugin-sdk-go/backend" @@ -18,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/metrics/metricutil" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/setting" @@ -200,6 +202,11 @@ func (hs *HTTPServer) DeleteDataSourceById(c *contextmodel.ReqContext) response. // 404: notFoundError // 500: internalServerError func (hs *HTTPServer) GetDataSourceByUID(c *contextmodel.ReqContext) response.Response { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(c.Req.Context(), hs.dsConfigHandlerRequestsDuration.WithLabelValues("legacy", "GetDataSourceByUID"), time.Since(start).Seconds()) + }() + ds, err := hs.getRawDataSourceByUID(c.Req.Context(), web.Params(c.Req)[":uid"], c.GetOrgID()) if err != nil { @@ -231,6 +238,11 @@ func (hs *HTTPServer) GetDataSourceByUID(c *contextmodel.ReqContext) response.Re // 404: notFoundError // 500: internalServerError func (hs *HTTPServer) DeleteDataSourceByUID(c *contextmodel.ReqContext) response.Response { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(c.Req.Context(), hs.dsConfigHandlerRequestsDuration.WithLabelValues("legacy", "DeleteDataSourceByUID"), time.Since(start).Seconds()) + }() + uid := web.Params(c.Req)[":uid"] if uid == "" { @@ -361,6 +373,11 @@ func validateJSONData(jsonData *simplejson.Json, cfg *setting.Cfg) error { // 409: conflictError // 500: internalServerError func (hs *HTTPServer) AddDataSource(c *contextmodel.ReqContext) response.Response { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(c.Req.Context(), hs.dsConfigHandlerRequestsDuration.WithLabelValues("legacy", "AddDataSource"), time.Since(start).Seconds()) + }() + cmd := datasources.AddDataSourceCommand{} if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) @@ -478,6 +495,10 @@ func (hs *HTTPServer) UpdateDataSourceByID(c *contextmodel.ReqContext) response. // 409: conflictError // 500: internalServerError func (hs *HTTPServer) UpdateDataSourceByUID(c *contextmodel.ReqContext) response.Response { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(c.Req.Context(), hs.dsConfigHandlerRequestsDuration.WithLabelValues("legacy", "UpdateDataSourceByUID"), time.Since(start).Seconds()) + }() cmd := datasources.UpdateDataSourceCommand{} if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) diff --git a/pkg/api/datasources_test.go b/pkg/api/datasources_test.go index 517dc6e9048..e2f951b0a37 100644 --- a/pkg/api/datasources_test.go +++ b/pkg/api/datasources_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -16,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db/dbtest" + "github.com/grafana/grafana/pkg/infra/metrics/metricutil" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" @@ -81,6 +83,19 @@ func TestDataSourcesProxy_userLoggedIn(t *testing.T) { }, mockSQLStore) } +// setupDsConfigMetrics creates and registers the prometheus metrics needed for HTTPServer tests +// that call methods using dsConfigHandlerRequestsDuration. +func setupDsConfigHandlerMetrics() (prometheus.Registerer, *prometheus.HistogramVec) { + promRegister := prometheus.NewRegistry() + dsConfigHandlerRequestsDuration := metricutil.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: "grafana", + Name: "ds_config_handler_requests_duration_seconds", + Help: "Duration of requests handled by datasource configuration handlers", + }, []string{"code_path", "handler"}) + promRegister.MustRegister(dsConfigHandlerRequestsDuration) + return promRegister, dsConfigHandlerRequestsDuration +} + // Adding data sources with invalid URLs should lead to an error. func TestAddDataSource_InvalidURL(t *testing.T) { sc := setupScenarioContext(t, "/api/datasources") @@ -88,6 +103,7 @@ func TestAddDataSource_InvalidURL(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{}, Cfg: setting.NewCfg(), } + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() sc.m.Post(sc.url, routing.Wrap(func(c *contextmodel.ReqContext) response.Response { c.Req.Body = mockRequestBody(datasources.AddDataSourceCommand{ @@ -118,6 +134,7 @@ func TestAddDataSource_URLWithoutProtocol(t *testing.T) { AccessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures()), accesscontrolService: actest.FakeService{}, } + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() sc := setupScenarioContext(t, "/api/datasources") @@ -143,6 +160,7 @@ func TestAddDataSource_InvalidJSONData(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{}, Cfg: setting.NewCfg(), } + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() sc := setupScenarioContext(t, "/api/datasources") @@ -175,6 +193,7 @@ func TestUpdateDataSource_InvalidURL(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{}, Cfg: setting.NewCfg(), } + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() sc := setupScenarioContext(t, "/api/datasources/1234") sc.m.Put(sc.url, routing.Wrap(func(c *contextmodel.ReqContext) response.Response { @@ -199,6 +218,7 @@ func TestUpdateDataSource_InvalidJSONData(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{}, Cfg: setting.NewCfg(), } + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() sc := setupScenarioContext(t, "/api/datasources/1234") hs.Cfg.AuthProxy.Enabled = true @@ -236,6 +256,7 @@ func TestAddDataSourceTeamHTTPHeaders(t *testing.T) { ExpectedErr: nil, }, } + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() sc := setupScenarioContext(t, fmt.Sprintf("/api/datasources/%s", tenantID)) hs.Cfg.AuthProxy.Enabled = true @@ -289,6 +310,7 @@ func TestUpdateDataSource_URLWithoutProtocol(t *testing.T) { AccessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures()), accesscontrolService: actest.FakeService{}, } + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() sc := setupScenarioContext(t, "/api/datasources/1234") @@ -429,6 +451,7 @@ func TestAPI_datasources_AccessControl(t *testing.T) { hs.DataSourcesService = &dataSourcesServiceMock{expectedDatasource: &datasources.DataSource{}} hs.accesscontrolService = actest.FakeService{} hs.Live = newTestLive(t, hs.SQLStore) + hs.promRegister, hs.dsConfigHandlerRequestsDuration = setupDsConfigHandlerMetrics() }) for _, url := range tt.urls { diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index fa0b407aa2b..f2ac32a80c6 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -203,27 +203,28 @@ type HTTPServer struct { pluginsCDNService *pluginscdn.Service managedPluginsService managedplugins.Manager - userService user.Service - tempUserService tempUser.Service - loginAttemptService loginAttempt.Service - orgService org.Service - orgDeletionService org.DeletionService - TeamService team.Service - accesscontrolService accesscontrol.Service - annotationsRepo annotations.Repository - tagService tag.Service - oauthTokenService oauthtoken.OAuthTokenService - statsService stats.Service - authnService authn.Service - starApi *starApi.API - promRegister prometheus.Registerer - promGatherer prometheus.Gatherer - clientConfigProvider grafanaapiserver.DirectRestConfigProvider - namespacer request.NamespaceMapper - anonService anonymous.Service - userVerifier user.Verifier - tlsCerts TLSCerts - htmlHandlerRequestsDuration *prometheus.HistogramVec + userService user.Service + tempUserService tempUser.Service + loginAttemptService loginAttempt.Service + orgService org.Service + orgDeletionService org.DeletionService + TeamService team.Service + accesscontrolService accesscontrol.Service + annotationsRepo annotations.Repository + tagService tag.Service + oauthTokenService oauthtoken.OAuthTokenService + statsService stats.Service + authnService authn.Service + starApi *starApi.API + promRegister prometheus.Registerer + promGatherer prometheus.Gatherer + clientConfigProvider grafanaapiserver.DirectRestConfigProvider + namespacer request.NamespaceMapper + anonService anonymous.Service + userVerifier user.Verifier + tlsCerts TLSCerts + htmlHandlerRequestsDuration *prometheus.HistogramVec + dsConfigHandlerRequestsDuration *prometheus.HistogramVec } type TLSCerts struct { @@ -382,9 +383,15 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi Name: "html_handler_requests_duration_seconds", Help: "Duration of requests handled by the index.go HTML handler", }, []string{"handler"}), + dsConfigHandlerRequestsDuration: metricutil.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: "grafana", + Name: "ds_config_handler_requests_duration_seconds", + Help: "Duration of requests handled by datasource configuration handlers", + }, []string{"code_path", "handler"}), } promRegister.MustRegister(hs.htmlHandlerRequestsDuration) + promRegister.MustRegister(hs.dsConfigHandlerRequestsDuration) if hs.Listener != nil { hs.log.Debug("Using provided listener") diff --git a/pkg/registry/apis/datasource/legacy_store.go b/pkg/registry/apis/datasource/legacy_store.go index 8786dc8d86e..aea7ed1daa3 100644 --- a/pkg/registry/apis/datasource/legacy_store.go +++ b/pkg/registry/apis/datasource/legacy_store.go @@ -3,7 +3,9 @@ package datasource import ( "context" "fmt" + "time" + "github.com/prometheus/client_golang/prometheus" "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -11,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apis/datasource/v0alpha1" + "github.com/grafana/grafana/pkg/infra/metrics/metricutil" ) var ( @@ -26,8 +29,9 @@ var ( ) type legacyStorage struct { - datasources PluginDatasourceProvider - resourceInfo *utils.ResourceInfo + datasources PluginDatasourceProvider + resourceInfo *utils.ResourceInfo + dsConfigHandlerRequestsDuration *prometheus.HistogramVec } func (s *legacyStorage) New() runtime.Object { @@ -57,11 +61,21 @@ func (s *legacyStorage) List(ctx context.Context, options *internalversion.ListO } func (s *legacyStorage) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(ctx, s.dsConfigHandlerRequestsDuration.WithLabelValues("new", "Get"), time.Since(start).Seconds()) + }() + return s.datasources.GetDataSource(ctx, name) } // Create implements rest.Creater. func (s *legacyStorage) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(ctx, s.dsConfigHandlerRequestsDuration.WithLabelValues("new", "Create"), time.Since(start).Seconds()) + }() + ds, ok := obj.(*v0alpha1.DataSource) if !ok { return nil, fmt.Errorf("expected a datasource object") @@ -71,6 +85,11 @@ func (s *legacyStorage) Create(ctx context.Context, obj runtime.Object, createVa // Update implements rest.Updater. func (s *legacyStorage) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(ctx, s.dsConfigHandlerRequestsDuration.WithLabelValues("new", "Create"), time.Since(start).Seconds()) + }() + old, err := s.Get(ctx, name, &metav1.GetOptions{}) if err != nil { return nil, false, err @@ -107,6 +126,11 @@ func (s *legacyStorage) Update(ctx context.Context, name string, objInfo rest.Up // Delete implements rest.GracefulDeleter. func (s *legacyStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { + start := time.Now() + defer func() { + metricutil.ObserveWithExemplar(ctx, s.dsConfigHandlerRequestsDuration.WithLabelValues("new", "Create"), time.Since(start).Seconds()) + }() + err := s.datasources.DeleteDataSource(ctx, name) return nil, false, err } diff --git a/pkg/registry/apis/datasource/register.go b/pkg/registry/apis/datasource/register.go index 9feb46a33d4..bec8fdaec5b 100644 --- a/pkg/registry/apis/datasource/register.go +++ b/pkg/registry/apis/datasource/register.go @@ -20,6 +20,7 @@ import ( datasourceV0 "github.com/grafana/grafana/pkg/apis/datasource/v0alpha1" queryV0 "github.com/grafana/grafana/pkg/apis/query/v0alpha1" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" + "github.com/grafana/grafana/pkg/infra/metrics/metricutil" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/manager/sources" "github.com/grafana/grafana/pkg/promlib/models" @@ -218,6 +219,11 @@ func (b *DataSourceAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver legacyStore := &legacyStorage{ datasources: b.datasources, resourceInfo: &ds, + dsConfigHandlerRequestsDuration: metricutil.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: "grafana", + Name: "ds_config_handler_requests_duration_seconds", + Help: "Duration of requests handled by datasource configuration handlers", + }, []string{"code_path", "handler"}), } unified, err := grafanaregistry.NewRegistryStore(opts.Scheme, ds, opts.OptsGetter) if err != nil {