ModifyQuery: Improve typing for modifyQuery and query hints (#52326)

* ModifyQuery: Improve typing

* Elasticsearch: Added `modifyQuery` method to add filters in Explore (#52313)

* fixed elasticsearch `QueryFixAction` type

Co-authored-by: Sven Grossmann <Svennergr@gmail.com>

.betterer.results

@@ -7685,8 +7685,7 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "3"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "4"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "5"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "6"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "6"]
-      [0, 0, 0, "Unexpected any. Specify a different type.", "7"]
     ],
     "public/app/plugins/datasource/loki/getDerivedFields.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
@@ -8246,8 +8245,7 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "32"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "33"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "34"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "35"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "35"]
-      [0, 0, 0, "Unexpected any. Specify a different type.", "36"]
     ],
     "public/app/plugins/datasource/prometheus/language_provider.test.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

packages/grafana-data/src/types/datasource.ts

@@ -506,6 +506,7 @@ export interface QueryFixAction {
   type: string;
   query?: string;
   preventSubmit?: boolean;
+  options?: KeyValue<string>;
 }
 
 export interface QueryHint {

public/app/features/explore/Explore.tsx

@@ -150,11 +150,11 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
   };
 
   onClickFilterLabel = (key: string, value: string) => {
-    this.onModifyQueries({ type: 'ADD_FILTER', key, value });
+    this.onModifyQueries({ type: 'ADD_FILTER', options: { key, value } });
   };
 
   onClickFilterOutLabel = (key: string, value: string) => {
-    this.onModifyQueries({ type: 'ADD_FILTER_OUT', key, value });
+    this.onModifyQueries({ type: 'ADD_FILTER_OUT', options: { key, value } });
   };
 
   onClickAddQueryRowButton = () => {

public/app/plugins/datasource/elasticsearch/datasource.test.ts

@@ -1033,15 +1033,19 @@ describe('modifyQuery', () => {
     });
 
     it('should add the filter', () => {
-      expect(ds.modifyQuery(query, { type: 'ADD_FILTER', key: 'foo', value: 'bar' }).query).toBe('foo:"bar"');
+      expect(ds.modifyQuery(query, { type: 'ADD_FILTER', options: { key: 'foo', value: 'bar' } }).query).toBe(
+        'foo:"bar"'
+      );
     });
 
     it('should add the negative filter', () => {
-      expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', key: 'foo', value: 'bar' }).query).toBe('-foo:"bar"');
+      expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe(
+        '-foo:"bar"'
+      );
     });
 
     it('should do nothing on unknown type', () => {
-      expect(ds.modifyQuery(query, { type: 'unknown', key: 'foo', value: 'bar' }).query).toBe(query.query);
+      expect(ds.modifyQuery(query, { type: 'unknown', options: { key: 'foo', value: 'bar' } }).query).toBe(query.query);
     });
   });
 
@@ -1052,19 +1056,19 @@ describe('modifyQuery', () => {
     });
 
     it('should add the filter', () => {
-      expect(ds.modifyQuery(query, { type: 'ADD_FILTER', key: 'foo', value: 'bar' }).query).toBe(
+      expect(ds.modifyQuery(query, { type: 'ADD_FILTER', options: { key: 'foo', value: 'bar' } }).query).toBe(
         'test:"value" AND foo:"bar"'
       );
     });
 
     it('should add the negative filter', () => {
-      expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', key: 'foo', value: 'bar' }).query).toBe(
+      expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe(
         'test:"value" AND -foo:"bar"'
       );
     });
 
     it('should do nothing on unknown type', () => {
-      expect(ds.modifyQuery(query, { type: 'unknown', key: 'foo', value: 'bar' }).query).toBe(query.query);
+      expect(ds.modifyQuery(query, { type: 'unknown', options: { key: 'foo', value: 'bar' } }).query).toBe(query.query);
     });
   });
 });

public/app/plugins/datasource/elasticsearch/datasource.ts

@@ -25,6 +25,7 @@ import {
   ScopedVars,
   TimeRange,
   toUtc,
+  QueryFixAction,
 } from '@grafana/data';
 import { BackendSrvRequest, getBackendSrv, getDataSourceSrv } from '@grafana/runtime';
 import { RowContextOptions } from '@grafana/ui/src/components/Logs/LogRowContextProvider';
@@ -953,21 +954,25 @@ export class ElasticDatasource
     return false;
   }
 
-  modifyQuery(query: ElasticsearchQuery, action: { type: string; key: string; value: string }): ElasticsearchQuery {
+  modifyQuery(query: ElasticsearchQuery, action: QueryFixAction): ElasticsearchQuery {
+    if (!action.options) {
+      return query;
+    }
+
     let expression = query.query ?? '';
     switch (action.type) {
       case 'ADD_FILTER': {
         if (expression.length > 0) {
           expression += ' AND ';
         }
-        expression += `${action.key}:"${action.value}"`;
+        expression += `${action.options.key}:"${action.options.value}"`;
         break;
       }
       case 'ADD_FILTER_OUT': {
         if (expression.length > 0) {
           expression += ' AND ';
         }
-        expression += `-${action.key}:"${action.value}"`;
+        expression += `-${action.options.key}:"${action.options.value}"`;
         break;
       }
     }

public/app/plugins/datasource/loki/datasource.test.ts

@@ -522,7 +522,7 @@ describe('LokiDatasource', () => {
       describe('and query has no parser', () => {
         it('then the correct label should be added for logs query', () => {
           const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' };
-          const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' };
+          const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
           const ds = createLokiDSForTests();
           const result = ds.modifyQuery(query, action);
 
@@ -532,7 +532,7 @@ describe('LokiDatasource', () => {
 
         it('then the correctly escaped label should be added for logs query', () => {
           const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' };
-          const action = { key: 'job', value: '\\test', type: 'ADD_FILTER' };
+          const action = { options: { key: 'job', value: '\\test' }, type: 'ADD_FILTER' };
           const ds = createLokiDSForTests();
           const result = ds.modifyQuery(query, action);
 
@@ -542,7 +542,7 @@ describe('LokiDatasource', () => {
 
         it('then the correct label should be added for metrics query', () => {
           const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"}[5m])' };
-          const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' };
+          const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
           const ds = createLokiDSForTests();
           const result = ds.modifyQuery(query, action);
 
@@ -552,7 +552,7 @@ describe('LokiDatasource', () => {
         describe('and query has parser', () => {
           it('then the correct label should be added for logs query', () => {
             const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' };
-            const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' };
+            const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
             const ds = createLokiDSForTests();
             const result = ds.modifyQuery(query, action);
 
@@ -561,7 +561,7 @@ describe('LokiDatasource', () => {
           });
           it('then the correct label should be added for metrics query', () => {
             const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' };
-            const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' };
+            const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' };
             const ds = createLokiDSForTests();
             const result = ds.modifyQuery(query, action);
 
@@ -576,7 +576,7 @@ describe('LokiDatasource', () => {
       describe('and query has no parser', () => {
         it('then the correct label should be added for logs query', () => {
           const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' };
-          const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' };
+          const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
           const ds = createLokiDSForTests();
           const result = ds.modifyQuery(query, action);
 
@@ -586,7 +586,7 @@ describe('LokiDatasource', () => {
 
         it('then the correctly escaped label should be added for logs query', () => {
           const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' };
-          const action = { key: 'job', value: '"test', type: 'ADD_FILTER_OUT' };
+          const action = { options: { key: 'job', value: '"test' }, type: 'ADD_FILTER_OUT' };
           const ds = createLokiDSForTests();
           const result = ds.modifyQuery(query, action);
 
@@ -596,7 +596,7 @@ describe('LokiDatasource', () => {
 
         it('then the correct label should be added for metrics query', () => {
           const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"}[5m])' };
-          const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' };
+          const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
           const ds = createLokiDSForTests();
           const result = ds.modifyQuery(query, action);
 
@@ -606,7 +606,7 @@ describe('LokiDatasource', () => {
         describe('and query has parser', () => {
           it('then the correct label should be added for logs query', () => {
             const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' };
-            const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' };
+            const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
             const ds = createLokiDSForTests();
             const result = ds.modifyQuery(query, action);
 
@@ -615,7 +615,7 @@ describe('LokiDatasource', () => {
           });
           it('then the correct label should be added for metrics query', () => {
             const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' };
-            const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' };
+            const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' };
             const ds = createLokiDSForTests();
             const result = ds.modifyQuery(query, action);
 

public/app/plugins/datasource/loki/datasource.ts

@@ -32,6 +32,7 @@ import {
   toUtc,
   QueryHint,
   getDefaultTimeRange,
+  QueryFixAction,
 } from '@grafana/data';
 import { FetchError, config, DataSourceWithBackend } from '@grafana/runtime';
 import { RowContextOptions } from '@grafana/ui/src/components/Logs/LogRowContextProvider';
@@ -389,15 +390,19 @@ export class LokiDatasource
     return escapedValues.join('|');
   }
 
-  modifyQuery(query: LokiQuery, action: any): LokiQuery {
+  modifyQuery(query: LokiQuery, action: QueryFixAction): LokiQuery {
     let expression = query.expr ?? '';
     switch (action.type) {
       case 'ADD_FILTER': {
-        expression = this.addLabelToQuery(expression, action.key, '=', action.value);
+        if (action.options?.key && action.options?.value) {
+          expression = this.addLabelToQuery(expression, action.options.key, '=', action.options.value);
+        }
         break;
       }
       case 'ADD_FILTER_OUT': {
-        expression = this.addLabelToQuery(expression, action.key, '!=', action.value);
+        if (action.options?.key && action.options?.value) {
+          expression = this.addLabelToQuery(expression, action.options.key, '!=', action.options.value);
+        }
         break;
       }
       case 'ADD_LOGFMT_PARSER': {

public/app/plugins/datasource/prometheus/components/PromQueryField.tsx

@@ -224,8 +224,9 @@ class PromQueryField extends React.PureComponent<PromQueryFieldProps, PromQueryF
   onClickHintFix = () => {
     const { datasource, query, onChange, onRunQuery } = this.props;
     const { hint } = this.state;
-
+    if (hint?.fix?.action) {
-    onChange(datasource.modifyQuery(query, hint!.fix!.action));
+      onChange(datasource.modifyQuery(query, hint.fix.action));
+    }
     onRunQuery();
   };
 

public/app/plugins/datasource/prometheus/datasource.test.ts

@@ -2154,7 +2154,7 @@ describe('modifyQuery', () => {
     describe('and query has no labels', () => {
       it('then the correct label should be added', () => {
         const query: PromQuery = { refId: 'A', expr: 'go_goroutines' };
-        const action = { key: 'cluster', value: 'us-cluster', type: 'ADD_FILTER' };
+        const action = { options: { key: 'cluster', value: 'us-cluster' }, type: 'ADD_FILTER' };
         const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings<PromOptions>;
         const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any);
 
@@ -2168,7 +2168,7 @@ describe('modifyQuery', () => {
     describe('and query has labels', () => {
       it('then the correct label should be added', () => {
         const query: PromQuery = { refId: 'A', expr: 'go_goroutines{cluster="us-cluster"}' };
-        const action = { key: 'pod', value: 'pod-123', type: 'ADD_FILTER' };
+        const action = { options: { key: 'pod', value: 'pod-123' }, type: 'ADD_FILTER' };
         const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings<PromOptions>;
         const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any);
 
@@ -2184,7 +2184,7 @@ describe('modifyQuery', () => {
     describe('and query has no labels', () => {
       it('then the correct label should be added', () => {
         const query: PromQuery = { refId: 'A', expr: 'go_goroutines' };
-        const action = { key: 'cluster', value: 'us-cluster', type: 'ADD_FILTER_OUT' };
+        const action = { options: { key: 'cluster', value: 'us-cluster' }, type: 'ADD_FILTER_OUT' };
         const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings<PromOptions>;
         const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any);
 
@@ -2198,7 +2198,7 @@ describe('modifyQuery', () => {
     describe('and query has labels', () => {
       it('then the correct label should be added', () => {
         const query: PromQuery = { refId: 'A', expr: 'go_goroutines{cluster="us-cluster"}' };
-        const action = { key: 'pod', value: 'pod-123', type: 'ADD_FILTER_OUT' };
+        const action = { options: { key: 'pod', value: 'pod-123' }, type: 'ADD_FILTER_OUT' };
         const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings<PromOptions>;
         const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any);
 

public/app/plugins/datasource/prometheus/datasource.tsx

@@ -22,6 +22,7 @@ import {
   TimeRange,
   DataFrame,
   dateTime,
+  QueryFixAction,
 } from '@grafana/data';
 import {
   BackendSrvRequest,
@@ -1030,15 +1031,22 @@ export class PrometheusDatasource
     }
   }
 
-  modifyQuery(query: PromQuery, action: any): PromQuery {
+  modifyQuery(query: PromQuery, action: QueryFixAction): PromQuery {
     let expression = query.expr ?? '';
     switch (action.type) {
       case 'ADD_FILTER': {
-        expression = addLabelToQuery(expression, action.key, action.value);
+        const { key, value } = action.options ?? {};
+        if (key && value) {
+          expression = addLabelToQuery(expression, key, value);
+        }
+
         break;
       }
       case 'ADD_FILTER_OUT': {
-        expression = addLabelToQuery(expression, action.key, action.value, '!=');
+        const { key, value } = action.options ?? {};
+        if (key && value) {
+          expression = addLabelToQuery(expression, key, value, '!=');
+        }
         break;
       }
       case 'ADD_HISTOGRAM_QUANTILE': {
@@ -1054,8 +1062,8 @@ export class PrometheusDatasource
         break;
       }
       case 'EXPAND_RULES': {
-        if (action.mapping) {
+        if (action.options) {
-          expression = expandRecordingRules(expression, action.mapping);
+          expression = expandRecordingRules(expression, action.options);
         }
         break;
       }

public/app/plugins/datasource/prometheus/query_hints.ts

@@ -103,7 +103,7 @@ export function getQueryHints(query: string, series?: any[], datasource?: Promet
           action: {
             type: 'EXPAND_RULES',
             query,
-            mapping: mappingForQuery,
+            options: mappingForQuery,
           },
         } as any as QueryFix,
       });

public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx

@@ -45,10 +45,12 @@ export const QueryBuilderHints = <T extends PromLokiVisualQuery>({
               <Tooltip content={`${hint.label} ${hint.fix?.label}`} key={hint.type}>
                 <Button
                   onClick={() => {
+                    if (hint?.fix?.action) {
                       const query = { expr: queryModeller.renderQuery(visualQuery), refId: '' };
-                    const newQuery = datasource.modifyQuery(query, hint!.fix!.action);
+                      const newQuery = datasource.modifyQuery(query, hint.fix.action);
                       const newVisualQuery = buildVisualQueryFromString(newQuery.expr);
                       return onChange(newVisualQuery.query);
+                    }
                   }}
                   fill="outline"
                   size="sm"