diff --git a/.betterer.results b/.betterer.results index a0bf6bdbc46..3b1d4fa728d 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3881,10 +3881,6 @@ exports[`better eslint`] = { "public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] ], - "public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/Alias.tsx:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"] - ], "public/app/plugins/datasource/cloudwatch/datasource.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/Alias.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/Alias.tsx deleted file mode 100644 index ba9d961352d..00000000000 --- a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/Alias.tsx +++ /dev/null @@ -1,47 +0,0 @@ -import { debounce } from 'lodash'; -import React, { useState } from 'react'; - -import { IconButton, Input, Tooltip } from '@grafana/ui'; - -export interface Props { - onChange: (alias: any) => void; - value: string; - id?: string; -} - -export const Alias = ({ value = '', onChange, id }: Props) => { - const [alias, setAlias] = useState(value); - - const propagateOnChange = debounce(onChange, 1500); - - onChange = (e: any) => { - setAlias(e.target.value); - propagateOnChange(e.target.value); - }; - - return ( -
- - - Alias pattern will be deprecated in Grafana 10. See{' '} - - documentation - {' '} - for how to use dynamic labels. - - } - interactive - theme="error" - placement="right" - > - - -
- ); -}; diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx index ca3056303b9..5b284abb0c4 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx @@ -111,27 +111,22 @@ describe('QueryEditor', () => { }); }); - describe('when dynamic labels feature toggle is enabled', () => { - it('should render label field', async () => { - const props = setup(); - const originalValue = config.featureToggles.cloudWatchDynamicLabels; - config.featureToggles.cloudWatchDynamicLabels = true; + it('should render label field and not alias field', async () => { + const props = setup(); - render( - - ); + render( + + ); - expect(await screen.findByText('Label')).toBeInTheDocument(); - expect(screen.queryByText('Alias')).toBeNull(); - expect(screen.getByText("Period: ${PROP('Period')} InstanceId: ${PROP('Dim.InstanceId')}")); - - config.featureToggles.cloudWatchDynamicLabels = originalValue; - }); + expect(await screen.findByText('Label')).toBeInTheDocument(); + expect(screen.queryByText('Alias')).toBeNull(); + expect(screen.getByText("Period: ${PROP('Period')} InstanceId: ${PROP('Dim.InstanceId')}")); }); + // TODO: delete this test when dynamic labels feature flag is removed describe('when dynamic labels feature toggle is disabled', () => { it('should render alias field', async () => { const props = setup(); @@ -141,9 +136,9 @@ describe('QueryEditor', () => { const expected = 'Period: {{period}} InstanceId: {{InstanceId}}'; render(); - expect(await screen.findByText('Alias')).toBeInTheDocument(); - expect(screen.queryByText('Label')).toBeNull(); - expect(screen.getByLabelText('Alias - optional')).toHaveValue(expected); + expect(await screen.findByText('Label')).toBeInTheDocument(); + expect(screen.queryByText('Alias')).toBeNull(); + expect(screen.getByText("Period: ${PROP('Period')} InstanceId: ${PROP('Dim.InstanceId')}")); config.featureToggles.cloudWatchDynamicLabels = originalValue; }); diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx index 3f53bd51c42..e6196131ea2 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx @@ -2,7 +2,6 @@ import React, { ChangeEvent, useCallback, useEffect, useState } from 'react'; import { QueryEditorProps, SelectableValue } from '@grafana/data'; import { EditorField, EditorRow, InlineSelect, Space } from '@grafana/experimental'; -import { config } from '@grafana/runtime'; import { ConfirmModal, Input, RadioButtonGroup } from '@grafana/ui'; import { MathExpressionQueryField, MetricStatEditor, SQLBuilderEditor } from '../'; @@ -19,8 +18,6 @@ import { import { DynamicLabelsField } from '../DynamicLabelsField'; import { SQLCodeEditor } from '../SQLCodeEditor'; -import { Alias } from './Alias'; - export interface Props extends QueryEditorProps { query: CloudWatchMetricsQuery; extraHeaderElementLeft?: React.Dispatch; @@ -182,28 +179,18 @@ export const MetricsQueryEditor = (props: Props) => { /> - {config.featureToggles.cloudWatchDynamicLabels ? ( - - props.onChange({ ...query, label })} - > - - ) : ( - - onChange({ ...migratedQuery, alias: value })} - /> - - )} + + props.onChange({ ...query, label })} + > + ); diff --git a/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.test.ts b/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.test.ts index 41122a8e66d..db9911fe679 100644 --- a/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.test.ts +++ b/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.test.ts @@ -25,68 +25,87 @@ describe('metricQueryMigrations', () => { matchExact: false, expression: '', }; - describe('when feature is enabled', () => { - describe('and label was not previously migrated', () => { - const cases: TestScenario[] = [ - { description: 'Metric name', alias: '{{metric}}', label: "${PROP('MetricName')}" }, - { description: 'Trim pattern', alias: '{{ metric }}', label: "${PROP('MetricName')}" }, - { description: 'Namespace', alias: '{{namespace}}', label: "${PROP('Namespace')}" }, - { description: 'Period', alias: '{{period}}', label: "${PROP('Period')}" }, - { description: 'Region', alias: '{{region}}', label: "${PROP('Region')}" }, - { description: 'Statistic', alias: '{{stat}}', label: "${PROP('Stat')}" }, - { description: 'Label', alias: '{{label}}', label: '${LABEL}' }, - { - description: 'Non-existing alias pattern', - alias: '{{anything_else}}', - label: "${PROP('Dim.anything_else')}", - }, - { - description: 'Combined patterns', - alias: 'some {{combination}} of {{label}} and {{metric}}', - label: "some ${PROP('Dim.combination')} of ${LABEL} and ${PROP('MetricName')}", - }, - { - description: 'Combined patterns not trimmed', - alias: 'some {{combination }}{{ label}} and {{metric}}', - label: "some ${PROP('Dim.combination')}${LABEL} and ${PROP('MetricName')}", - }, - ]; - test.each(cases)('given old alias %p, it should be migrated to label: %p', ({ alias, label }) => { - config.featureToggles.cloudWatchDynamicLabels = true; - const testQuery = { ...baseQuery, alias }; - const result = migrateAliasPatterns(testQuery); - expect(result.label).toBe(label); - expect(result.alias).toBe(alias); - }); - }); - - describe('and label was previously migrated', () => { - const cases: TestScenario[] = [ - { - alias: '', - label: "${PROP('MetricName')}", - }, - { alias: '{{metric}}', label: "${PROP('Period')}" }, - { alias: '{{namespace}}', label: '' }, - ]; - test.each(cases)('it should not be migrated once again', ({ alias, label }) => { - config.featureToggles.cloudWatchDynamicLabels = true; - const testQuery = { ...baseQuery, alias, label }; - const result = migrateAliasPatterns(testQuery); - expect(result.label).toBe(label); - expect(result.alias).toBe(alias); - }); + describe('when label was not previously migrated', () => { + const cases: TestScenario[] = [ + { description: 'Metric name', alias: '{{metric}}', label: "${PROP('MetricName')}" }, + { description: 'Trim pattern', alias: '{{ metric }}', label: "${PROP('MetricName')}" }, + { description: 'Namespace', alias: '{{namespace}}', label: "${PROP('Namespace')}" }, + { description: 'Period', alias: '{{period}}', label: "${PROP('Period')}" }, + { description: 'Region', alias: '{{region}}', label: "${PROP('Region')}" }, + { description: 'Statistic', alias: '{{stat}}', label: "${PROP('Stat')}" }, + { description: 'Label', alias: '{{label}}', label: '${LABEL}' }, + { + description: 'Non-existing alias pattern', + alias: '{{anything_else}}', + label: "${PROP('Dim.anything_else')}", + }, + { + description: 'Combined patterns', + alias: 'some {{combination}} of {{label}} and {{metric}}', + label: "some ${PROP('Dim.combination')} of ${LABEL} and ${PROP('MetricName')}", + }, + { + description: 'Combined patterns not trimmed', + alias: 'some {{combination }}{{ label}} and {{metric}}', + label: "some ${PROP('Dim.combination')}${LABEL} and ${PROP('MetricName')}", + }, + ]; + test.each(cases)('given old alias %p, it should be migrated to label: %p', ({ alias, label }) => { + const testQuery = { ...baseQuery, alias }; + const result = migrateAliasPatterns(testQuery); + expect(result.label).toBe(label); + expect(result.alias).toBe(alias); }); }); + describe('when label was previously migrated', () => { + const cases: TestScenario[] = [ + { + alias: '', + label: "${PROP('MetricName')}", + }, + { alias: '{{metric}}', label: "${PROP('Period')}" }, + { alias: '{{namespace}}', label: '' }, + ]; + test.each(cases)('it should not be migrated once again', ({ alias, label }) => { + const testQuery = { ...baseQuery, alias, label }; + const result = migrateAliasPatterns(testQuery); + expect(result.label).toBe(label); + expect(result.alias).toBe(alias); + }); + }); + + // TODO: delete this test when dynamic labels feature flag is removed describe('when feature is disabled', () => { const cases: TestScenario[] = [ { alias: '{{period}}', label: "${PROP('MetricName')}" }, - { alias: '{{metric}}', label: '' }, + { alias: '{{metric}}', label: "${PROP('MetricName')}" }, { alias: '', label: "${PROP('MetricName')}" }, - { alias: '', label: undefined }, + { alias: '', label: '' }, + { description: 'Metric name', alias: '{{metric}}', label: "${PROP('MetricName')}" }, + { description: 'Trim pattern', alias: '{{ metric }}', label: "${PROP('MetricName')}" }, + { description: 'Namespace', alias: '{{namespace}}', label: "${PROP('Namespace')}" }, + { description: 'Period', alias: '{{period}}', label: "${PROP('Period')}" }, + { description: 'Region', alias: '{{region}}', label: "${PROP('Region')}" }, + { description: 'Statistic', alias: '{{stat}}', label: "${PROP('Stat')}" }, + { description: 'Label', alias: '{{label}}', label: '${LABEL}' }, + { + description: 'Non-existing alias pattern', + alias: '{{anything_else}}', + label: "${PROP('Dim.anything_else')}", + }, + { + description: 'Combined patterns', + alias: 'some {{combination}} of {{label}} and {{metric}}', + label: "some ${PROP('Dim.combination')} of ${LABEL} and ${PROP('MetricName')}", + }, + { + description: 'Combined patterns not trimmed', + alias: 'some {{combination }}{{ label}} and {{metric}}', + label: "some ${PROP('Dim.combination')}${LABEL} and ${PROP('MetricName')}", + }, ]; - test.each(cases)('should not migrate alias', ({ alias, label }) => { + test.each(cases)('should migrate alias anyway', ({ alias, label }) => { config.featureToggles.cloudWatchDynamicLabels = false; const testQuery = { ...baseQuery, alias: `${alias}` }; if (label !== undefined) { diff --git a/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.ts b/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.ts index 0cffd1cdb6e..d734a234330 100644 --- a/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.ts +++ b/public/app/plugins/datasource/cloudwatch/migrations/metricQueryMigrations.ts @@ -1,7 +1,5 @@ import deepEqual from 'fast-deep-equal'; -import { config } from '@grafana/runtime'; - import { CloudWatchMetricsQuery } from '../types'; // Call this function to migrate queries from within the plugin. @@ -20,8 +18,9 @@ const aliasPatterns: Record = { label: `LABEL`, }; +// migrateAliasPatterns in the context of https://github.com/grafana/grafana/issues/48434 export function migrateAliasPatterns(query: CloudWatchMetricsQuery): CloudWatchMetricsQuery { - if (config.featureToggles.cloudWatchDynamicLabels && !query.hasOwnProperty('label')) { + if (!query.hasOwnProperty('label')) { const newQuery = { ...query }; if (!query.hasOwnProperty('label')) { const regex = /{{\s*(.+?)\s*}}/g; diff --git a/public/app/plugins/datasource/cloudwatch/migrations/useMigratedMetricsQuery.test.ts b/public/app/plugins/datasource/cloudwatch/migrations/useMigratedMetricsQuery.test.ts index d29fef5a1da..8459f146342 100644 --- a/public/app/plugins/datasource/cloudwatch/migrations/useMigratedMetricsQuery.test.ts +++ b/public/app/plugins/datasource/cloudwatch/migrations/useMigratedMetricsQuery.test.ts @@ -10,10 +10,9 @@ import useMigratedMetricsQuery from './useMigratedMetricsQuery'; describe('usePrepareMetricsQuery', () => { const DEFAULT_TEST_QUERY: CloudWatchMetricsQuery = { ...DEFAULT_METRICS_QUERY, refId: 'testId' }; - describe('when dynamic labels are true and there is no label', () => { + describe('when there is no label', () => { const testQuery: CloudWatchMetricsQuery = { ...DEFAULT_TEST_QUERY, alias: 'test' }; it('should replace label with alias and trigger onChangeQuery', async () => { - config.featureToggles.cloudWatchDynamicLabels = true; const expectedQuery: CloudWatchMetricsQuery = migrateAliasPatterns(testQuery); const onChangeQuery = jest.fn(); const { result } = renderHook(() => useMigratedMetricsQuery(testQuery, onChangeQuery)); @@ -31,14 +30,16 @@ describe('usePrepareMetricsQuery', () => { expect(onChangeQuery).toHaveBeenCalledTimes(0); }); }); + // TODO: delete this test when dynamic labels feature flag is removed describe('when dynamic labels feature flag is disabled', () => { const testQuery: CloudWatchMetricsQuery = { ...DEFAULT_TEST_QUERY }; - it('should not replace label or trigger onChange', async () => { + it('should replace label or trigger onChange', async () => { + const expectedQuery: CloudWatchMetricsQuery = migrateAliasPatterns(testQuery); config.featureToggles.cloudWatchDynamicLabels = false; const onChangeQuery = jest.fn(); const { result } = renderHook(() => useMigratedMetricsQuery(testQuery, onChangeQuery)); - expect(result.current).toEqual(testQuery); - expect(onChangeQuery).toHaveBeenCalledTimes(0); + expect(onChangeQuery).toHaveBeenLastCalledWith(result.current); + expect(result.current).toEqual(expectedQuery); }); }); });