From b1a6860f525707ca172c1e5ed96db8ed3b3c7276 Mon Sep 17 00:00:00 2001 From: Alexa V <239999+axelavargas@users.noreply.github.com> Date: Tue, 20 May 2025 17:55:30 +0200 Subject: [PATCH] Dashboards: Schema V2 - Rename Annotations 'options' to 'legacyOptions' and Fix stale props (#105266) * Rename Annotations 'Options' to 'legacyOptions' * use new legacyOptions in StandardAnnotationQueryEditor * use new legacyOptions in transformSaveModelSchemaToScene * use legacyOptions in transformSceneToSaveModelSchemaV2 * Fix bug with ds query editors not taking the latest state * update snapshot --- .../kinds/v2alpha1/dashboard_spec.cue | 2 +- .../v0alpha1/dashboard_object_gen.go | 2 + .../dashboard/v1beta1/dashboard_object_gen.go | 2 + .../dashboard/v2alpha1/dashboard_spec.cue | 2 +- .../dashboard/v2alpha1/dashboard_spec_gen.go | 2 +- .../v2alpha1/zz_generated.openapi.go | 2 +- apps/dashboard/pkg/apis/dashboard_manifest.go | 2 + apps/folder/pkg/apis/folder_manifest.go | 2 + .../dashboard/v2alpha1/types.spec.gen.ts | 2 +- .../dashboard.grafana.app-v2alpha1.json | 10 ++--- .../StandardAnnotationQueryEditor.test.tsx | 38 +++++++++---------- .../StandardAnnotationQueryEditor.tsx | 33 +++++++++++----- .../transformSaveModelSchemaV2ToScene.test.ts | 14 +++---- .../transformSaveModelSchemaV2ToScene.ts | 6 +-- .../transformSceneToSaveModelSchemaV2.test.ts | 10 ++--- .../transformSceneToSaveModelSchemaV2.ts | 11 +++--- 16 files changed, 82 insertions(+), 58 deletions(-) diff --git a/apps/dashboard/kinds/v2alpha1/dashboard_spec.cue b/apps/dashboard/kinds/v2alpha1/dashboard_spec.cue index 9f23a8e5b81..6bedd25edb7 100644 --- a/apps/dashboard/kinds/v2alpha1/dashboard_spec.cue +++ b/apps/dashboard/kinds/v2alpha1/dashboard_spec.cue @@ -393,7 +393,7 @@ AnnotationQuerySpec: { name: string builtIn?: bool | *false filter?: AnnotationPanelFilter - options?: [string]: _ //Catch-all field for datasource-specific properties + legacyOptions?: [string]: _ //Catch-all field for datasource-specific properties } AnnotationQueryKind: { diff --git a/apps/dashboard/pkg/apis/dashboard/v0alpha1/dashboard_object_gen.go b/apps/dashboard/pkg/apis/dashboard/v0alpha1/dashboard_object_gen.go index a267e0c8df8..50847df87c3 100644 --- a/apps/dashboard/pkg/apis/dashboard/v0alpha1/dashboard_object_gen.go +++ b/apps/dashboard/pkg/apis/dashboard/v0alpha1/dashboard_object_gen.go @@ -294,6 +294,8 @@ var _ resource.ListObject = &DashboardList{} // Copy methods for all subresource types + + // DeepCopy creates a full deep copy of DashboardStatus func (s *DashboardStatus) DeepCopy() *DashboardStatus { cpy := &DashboardStatus{} diff --git a/apps/dashboard/pkg/apis/dashboard/v1beta1/dashboard_object_gen.go b/apps/dashboard/pkg/apis/dashboard/v1beta1/dashboard_object_gen.go index be021b5f003..1423c7b0603 100644 --- a/apps/dashboard/pkg/apis/dashboard/v1beta1/dashboard_object_gen.go +++ b/apps/dashboard/pkg/apis/dashboard/v1beta1/dashboard_object_gen.go @@ -294,6 +294,8 @@ var _ resource.ListObject = &DashboardList{} // Copy methods for all subresource types + + // DeepCopy creates a full deep copy of DashboardStatus func (s *DashboardStatus) DeepCopy() *DashboardStatus { cpy := &DashboardStatus{} diff --git a/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec.cue b/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec.cue index ba2bd0990de..c6eb849f6bc 100644 --- a/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec.cue +++ b/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec.cue @@ -397,7 +397,7 @@ AnnotationQuerySpec: { name: string builtIn?: bool | *false filter?: AnnotationPanelFilter - options?: [string]: _ //Catch-all field for datasource-specific properties + legacyOptions?: [string]: _ //Catch-all field for datasource-specific properties } AnnotationQueryKind: { diff --git a/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec_gen.go b/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec_gen.go index 58e4396e8f6..19ccd04527d 100644 --- a/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec_gen.go +++ b/apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec_gen.go @@ -33,7 +33,7 @@ type DashboardAnnotationQuerySpec struct { BuiltIn *bool `json:"builtIn,omitempty"` Filter *DashboardAnnotationPanelFilter `json:"filter,omitempty"` // Catch-all field for datasource-specific properties - Options map[string]interface{} `json:"options,omitempty"` + LegacyOptions map[string]interface{} `json:"legacyOptions,omitempty"` } // NewDashboardAnnotationQuerySpec creates a new DashboardAnnotationQuerySpec object. diff --git a/apps/dashboard/pkg/apis/dashboard/v2alpha1/zz_generated.openapi.go b/apps/dashboard/pkg/apis/dashboard/v2alpha1/zz_generated.openapi.go index a206d843e59..20fd5a637cc 100644 --- a/apps/dashboard/pkg/apis/dashboard/v2alpha1/zz_generated.openapi.go +++ b/apps/dashboard/pkg/apis/dashboard/v2alpha1/zz_generated.openapi.go @@ -647,7 +647,7 @@ func schema_pkg_apis_dashboard_v2alpha1_DashboardAnnotationQuerySpec(ref common. Ref: ref("github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1.DashboardAnnotationPanelFilter"), }, }, - "options": { + "legacyOptions": { SchemaProps: spec.SchemaProps{ Description: "Catch-all field for datasource-specific properties", Type: []string{"object"}, diff --git a/apps/dashboard/pkg/apis/dashboard_manifest.go b/apps/dashboard/pkg/apis/dashboard_manifest.go index cc5451a78e8..e8ebdfbc5ed 100644 --- a/apps/dashboard/pkg/apis/dashboard_manifest.go +++ b/apps/dashboard/pkg/apis/dashboard_manifest.go @@ -11,6 +11,8 @@ import ( "github.com/grafana/grafana-app-sdk/app" ) +var () + var appManifestData = app.ManifestData{ AppName: "dashboard", Group: "dashboard.grafana.app", diff --git a/apps/folder/pkg/apis/folder_manifest.go b/apps/folder/pkg/apis/folder_manifest.go index 4d45b12e405..7a78226361c 100644 --- a/apps/folder/pkg/apis/folder_manifest.go +++ b/apps/folder/pkg/apis/folder_manifest.go @@ -11,6 +11,8 @@ import ( "github.com/grafana/grafana-app-sdk/app" ) +var () + var appManifestData = app.ManifestData{ AppName: "folder", Group: "folder.grafana.app", diff --git a/packages/grafana-schema/src/schema/dashboard/v2alpha1/types.spec.gen.ts b/packages/grafana-schema/src/schema/dashboard/v2alpha1/types.spec.gen.ts index 5956f2b61a8..009026fa970 100644 --- a/packages/grafana-schema/src/schema/dashboard/v2alpha1/types.spec.gen.ts +++ b/packages/grafana-schema/src/schema/dashboard/v2alpha1/types.spec.gen.ts @@ -20,7 +20,7 @@ export interface AnnotationQuerySpec { builtIn?: boolean; filter?: AnnotationPanelFilter; // Catch-all field for datasource-specific properties - options?: Record; + legacyOptions?: Record; } export const defaultAnnotationQuerySpec = (): AnnotationQuerySpec => ({ diff --git a/pkg/tests/apis/openapi_snapshots/dashboard.grafana.app-v2alpha1.json b/pkg/tests/apis/openapi_snapshots/dashboard.grafana.app-v2alpha1.json index bde9543fc20..de9b158d7c7 100644 --- a/pkg/tests/apis/openapi_snapshots/dashboard.grafana.app-v2alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/dashboard.grafana.app-v2alpha1.json @@ -1277,17 +1277,17 @@ "type": "string", "default": "" }, - "name": { - "type": "string", - "default": "" - }, - "options": { + "legacyOptions": { "description": "Catch-all field for datasource-specific properties", "type": "object", "additionalProperties": { "type": "object" } }, + "name": { + "type": "string", + "default": "" + }, "query": { "$ref": "#/components/schemas/com.github.grafana.grafana.apps.dashboard.pkg.apis.dashboard.v2alpha1.DashboardDataQueryKind" } diff --git a/public/app/features/annotations/components/StandardAnnotationQueryEditor.test.tsx b/public/app/features/annotations/components/StandardAnnotationQueryEditor.test.tsx index 3f693442bb1..d0738045a7f 100644 --- a/public/app/features/annotations/components/StandardAnnotationQueryEditor.test.tsx +++ b/public/app/features/annotations/components/StandardAnnotationQueryEditor.test.tsx @@ -68,14 +68,14 @@ describe('StandardAnnotationQueryEditor', () => { ); }); - it('v2 dashboard - should preserve options field when changing target', () => { - // Setup with annotation that has options + it('v2 dashboard - should preserve legacyOptions field when changing target', () => { + // Setup with annotation that has legacyOptions const mockOnChange = jest.fn(); const { props } = setup({ annotation: { - name: 'annotationWithOptions', + name: 'annotationWithLegacyOptions', target: { refId: 'refId1' }, - options: { + legacyOptions: { expr: 'rate(http_requests_total[5m])', queryType: 'range', }, @@ -98,11 +98,11 @@ describe('StandardAnnotationQueryEditor', () => { // Simulate changing the target componentInstance.onChange({ refId: 'refId2', newField: 'value' }); - // Check that options are preserved + // Check that legacyOptions are preserved expect(mockOnChange).toHaveBeenCalledWith( expect.objectContaining({ target: { refId: 'refId2', newField: 'value' }, - options: { + legacyOptions: { expr: 'rate(http_requests_total[5m])', queryType: 'range', }, @@ -110,13 +110,13 @@ describe('StandardAnnotationQueryEditor', () => { ); }); - it('should preserve options field when using onAnnotationChange', () => { - // Setup with annotation that has options + it('should preserve legacyOptions field when using onAnnotationChange', () => { + // Setup with annotation that has legacyOptions const mockOnChange = jest.fn(); const { props } = setup({ annotation: { - name: 'annotationWithOptions', - options: { + name: 'annotationWithLegacyOptions', + legacyOptions: { expr: 'rate(http_requests_total[5m])', queryType: 'range', }, @@ -142,12 +142,12 @@ describe('StandardAnnotationQueryEditor', () => { iconColor: 'red', }); - // Check that options are preserved + // Check that legacyOptions are preserved expect(mockOnChange).toHaveBeenCalledWith( expect.objectContaining({ name: 'newName', iconColor: 'red', - options: { + legacyOptions: { expr: 'rate(http_requests_total[5m])', queryType: 'range', }, @@ -187,17 +187,17 @@ describe('StandardAnnotationQueryEditor', () => { ); }); - it('should propagate options to root level for v2 dashboards', () => { + it('should propagate legacyOptions to root level for v2 dashboards', () => { const { props } = setup({ annotation: { - name: 'v2annotationWithOptions', + name: 'v2annotationWithLegacyOptions', query: { kind: 'prometheus', spec: { refId: 'A', }, }, - options: { + legacyOptions: { expr: 'rate(http_requests_total[5m])', legendFormat: '{{method}} {{endpoint}}', }, @@ -210,13 +210,13 @@ describe('StandardAnnotationQueryEditor', () => { } as unknown as DataSourceApi, }); - // Check that options are propagated to root level for the editor + // Check that legacyOptions are propagated to root level for the editor expect(props.datasource?.annotations?.QueryEditor).toHaveBeenCalledWith( expect.objectContaining({ annotation: expect.objectContaining({ - name: 'v2annotationWithOptions', + name: 'v2annotationWithLegacyOptions', query: expect.anything(), - options: expect.anything(), + legacyOptions: expect.anything(), expr: 'rate(http_requests_total[5m])', legendFormat: '{{method}} {{endpoint}}', }), @@ -235,7 +235,7 @@ describe('StandardAnnotationQueryEditor', () => { type: 'prometheus', uid: 'abc123', }, - // v1 dashboards don't have options field + // v1 dashboards don't have legacyOptions field enable: true, iconColor: 'red', hide: false, diff --git a/public/app/features/annotations/components/StandardAnnotationQueryEditor.tsx b/public/app/features/annotations/components/StandardAnnotationQueryEditor.tsx index 3fb6abe7338..1f6af567c78 100644 --- a/public/app/features/annotations/components/StandardAnnotationQueryEditor.tsx +++ b/public/app/features/annotations/components/StandardAnnotationQueryEditor.tsx @@ -101,11 +101,27 @@ export default class StandardAnnotationQueryEditor extends PureComponent { + // if dealing with v2 dashboards + if (this.props.annotation.query && this.props.annotation.query.spec) { + target = { + ...this.props.annotation.query.spec, + ...target, + }; + } + //target property is what ds query editor are using, but for v2 we also need to keep query in sync this.props.onChange({ ...this.props.annotation, + // the query editor uses target, but the annotation in v2 uses query + // therefore we need to keep the target and query in sync target, - // Keep options from the original annotation if they exist - ...(this.props.annotation.options ? { options: this.props.annotation.options } : {}), + ...(this.props.annotation.query && { + query: { + kind: this.props.annotation.query.kind, + spec: { ...target }, + }, + }), + // Keep legacyOptions from the original annotation if they exist + ...(this.props.annotation.legacyOptions ? { legacyOptions: this.props.annotation.legacyOptions } : {}), }); }; @@ -209,11 +225,11 @@ export default class StandardAnnotationQueryEditor extends PureComponent { - // Also preserve any options field that might exist when migrating from V2 to V1 + // Also preserve any legacyOptions field that might exist when migrating from V2 to V1 this.props.onChange({ ...annotation, - // Keep options from the original annotation if they exist - ...(this.props.annotation.options ? { options: this.props.annotation.options } : {}), + // Keep legacyOptions from the original annotation if they exist + ...(this.props.annotation.legacyOptions ? { legacyOptions: this.props.annotation.legacyOptions } : {}), }); }; @@ -251,10 +267,9 @@ export default class StandardAnnotationQueryEditor extends PureComponent { }); describe('annotations', () => { - it('should transform annotation with options field', () => { + it('should transform annotation with legacyOptions field', () => { // Create a dashboard with an annotation that has options const dashboardWithAnnotationOptions: DashboardWithAccessInfo = { kind: 'DashboardWithAccessInfo', @@ -721,7 +721,7 @@ describe('transformSaveModelSchemaV2ToScene', () => { { kind: 'AnnotationQuery', spec: { - name: 'Annotation with options', + name: 'Annotation with legacy options', builtIn: false, enable: true, hide: false, @@ -730,7 +730,7 @@ describe('transformSaveModelSchemaV2ToScene', () => { type: 'prometheus', uid: 'abc123', }, - options: { + legacyOptions: { expr: 'rate(http_requests_total[5m])', queryType: 'range', legendFormat: '{{method}} {{endpoint}}', @@ -773,9 +773,9 @@ describe('transformSaveModelSchemaV2ToScene', () => { const annotationLayer = dataLayerSet.state.annotationLayers[1] as DashboardAnnotationsDataLayer; - // Verify that the options have been merged into the query object + // Verify that the legacyOptions have been merged into the query object expect(annotationLayer.state.query).toMatchObject({ - name: 'Annotation with options', + name: 'Annotation with legacy options', expr: 'rate(http_requests_total[5m])', queryType: 'range', legendFormat: '{{method}} {{endpoint}}', @@ -783,8 +783,8 @@ describe('transformSaveModelSchemaV2ToScene', () => { step: '1m', }); - // Verify the original options object is also preserved - expect(annotationLayer.state.query.options).toMatchObject({ + // Verify the original legacyOptions object is also preserved + expect(annotationLayer.state.query.legacyOptions).toMatchObject({ expr: 'rate(http_requests_total[5m])', queryType: 'range', legendFormat: '{{method}} {{endpoint}}', diff --git a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts index df62ce83f26..dcc683771f0 100644 --- a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts +++ b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts @@ -98,12 +98,12 @@ export function transformSaveModelSchemaV2ToScene(dto: DashboardWithAccessInfo { let annoQuerySpec = annotation.spec; - // some annotations will contain in the options properties that need to be + // some annotations will contain in the legacyOptions properties that need to be // added to the root level annotation spec - if (annoQuerySpec?.options) { + if (annoQuerySpec?.legacyOptions) { annoQuerySpec = { ...annoQuerySpec, - ...annoQuerySpec.options, + ...annoQuerySpec.legacyOptions, }; } return new DashboardAnnotationsDataLayer({ diff --git a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.test.ts b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.test.ts index 19a646f64e8..e88afda7e4e 100644 --- a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.test.ts +++ b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.test.ts @@ -576,10 +576,10 @@ describe('transformSceneToSaveModelSchemaV2', () => { }); }); - it('should test annotation with options field', () => { + it('should test annotation with legacyOptions field', () => { // Create a scene with an annotation layer that has options const annotationWithOptions = new DashboardAnnotationsDataLayer({ - key: 'layerWithOptions', + key: 'layerWithLegacyOptions', query: { datasource: { type: 'prometheus', @@ -588,7 +588,7 @@ describe('transformSceneToSaveModelSchemaV2', () => { name: 'annotation-with-options', enable: true, iconColor: 'red', - options: { + legacyOptions: { expr: 'rate(http_requests_total[5m])', queryType: 'range', legendFormat: '{{method}} {{endpoint}}', @@ -617,8 +617,8 @@ describe('transformSceneToSaveModelSchemaV2', () => { // Verify the annotation options are properly serialized expect(result.annotations.length).toBe(1); - expect(result.annotations[0].spec.options).toBeDefined(); - expect(result.annotations[0].spec.options).toEqual({ + expect(result.annotations[0].spec.legacyOptions).toBeDefined(); + expect(result.annotations[0].spec.legacyOptions).toEqual({ expr: 'rate(http_requests_total[5m])', queryType: 'range', legendFormat: '{{method}} {{endpoint}}', diff --git a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts index f8b332bd2e9..b8a0bfd3b6e 100644 --- a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts +++ b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts @@ -420,7 +420,9 @@ function getAnnotations(state: DashboardSceneState, dsReferencesMapping?: DSRefe }; // Transform v1 dashboard (using target) to v2 structure - if (layer.state.query.target) { + // adds extra condition to prioritize query over target + // if query is defined, use it + if (layer.state.query.target && !layer.state.query.query) { // Handle built-in annotations if (layer.state.query.builtIn) { result.spec.query = { @@ -462,13 +464,12 @@ function getAnnotations(state: DashboardSceneState, dsReferencesMapping?: DSRefe 'query' ); - // Store extra properties in the options field instead of directly in the spec + // Store extra properties in the legacyOptions field instead of directly in the spec if (Object.keys(otherProps).length > 0) { // Extract options property and get the rest of the properties - const { options, ...restProps } = otherProps; - + const { legacyOptions, ...restProps } = otherProps; // Merge options with the rest of the properties - result.spec.options = { ...options, ...restProps }; + result.spec.legacyOptions = { ...legacyOptions, ...restProps }; } // If filter is an empty array, don't save it