Merge pull request #16006 from grafana/fix-explore-refId

Explore: Query row using character refId instead of number

public/app/core/utils/explore.ts

@@ -9,6 +9,7 @@ import store from 'app/core/store';
 import { parse as parseDate } from 'app/core/utils/datemath';
 import { colors } from '@grafana/ui';
 import TableModel, { mergeTablesIntoModel } from 'app/core/table_model';
+import { getNextRefIdChar } from './query';
 
 // Types
 import { RawTimeRange, IntervalValues, DataQuery, DataSourceApi } from '@grafana/ui';
@@ -225,12 +226,8 @@ export function generateKey(index = 0): string {
   return `Q-${Date.now()}-${Math.random()}-${index}`;
 }
 
-export function generateRefId(index = 0): string {
+export function generateEmptyQuery(queries: DataQuery[], index = 0): DataQuery {
-  return `${index + 1}`;
+  return { refId: getNextRefIdChar(queries), key: generateKey(index) };
-}
-
-export function generateEmptyQuery(index = 0): { refId: string; key: string } {
-  return { refId: generateRefId(index), key: generateKey(index) };
 }
 
 /**
@@ -238,9 +235,9 @@ export function generateEmptyQuery(index = 0): { refId: string; key: string } {
  */
 export function ensureQueries(queries?: DataQuery[]): DataQuery[] {
   if (queries && typeof queries === 'object' && queries.length > 0) {
-    return queries.map((query, i) => ({ ...query, ...generateEmptyQuery(i) }));
+    return queries.map((query, i) => ({ ...query, ...generateEmptyQuery(queries, i) }));
   }
-  return [{ ...generateEmptyQuery() }];
+  return [{ ...generateEmptyQuery(queries) }];
 }
 
 /**

public/app/core/utils/query.test.ts

@@ -0,0 +1,30 @@
+import { DataQuery } from '@grafana/ui';
+import { getNextRefIdChar } from './query';
+
+const dataQueries: DataQuery[] = [
+  {
+    refId: 'A',
+  },
+  {
+    refId: 'B',
+  },
+  {
+    refId: 'C',
+  },
+  {
+    refId: 'D',
+  },
+  {
+    refId: 'E',
+  },
+];
+
+describe('Get next refId char', () => {
+  it('should return next char', () => {
+    expect(getNextRefIdChar(dataQueries)).toEqual('F');
+  });
+
+  it('should get first char', () => {
+    expect(getNextRefIdChar([])).toEqual('A');
+  });
+});

public/app/core/utils/query.ts

@@ -0,0 +1,12 @@
+import _ from 'lodash';
+import { DataQuery } from '@grafana/ui/';
+
+export const getNextRefIdChar = (queries: DataQuery[]): string => {
+  const letters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
+
+  return _.find(letters, refId => {
+    return _.every(queries, other => {
+      return other.refId !== refId;
+    });
+  });
+};

public/app/features/dashboard/state/PanelModel.ts

@@ -1,8 +1,11 @@
 // Libraries
 import _ from 'lodash';
 
-// Types
+// Utils
 import { Emitter } from 'app/core/utils/emitter';
+import { getNextRefIdChar } from 'app/core/utils/query';
+
+// Types
 import { DataQuery, TimeSeries, Threshold, ScopedVars, PanelTypeChangedHook } from '@grafana/ui';
 import { TableData } from '@grafana/ui/src';
 
@@ -128,7 +131,7 @@ export class PanelModel {
     if (this.targets && _.isArray(this.targets)) {
       for (const query of this.targets) {
         if (!query.refId) {
-          query.refId = this.getNextQueryLetter();
+          query.refId = getNextRefIdChar(this.targets);
         }
       }
     }
@@ -266,20 +269,10 @@ export class PanelModel {
 
   addQuery(query?: Partial<DataQuery>) {
     query = query || { refId: 'A' };
-    query.refId = this.getNextQueryLetter();
+    query.refId = getNextRefIdChar(this.targets);
     this.targets.push(query as DataQuery);
   }
 
-  getNextQueryLetter(): string {
-    const letters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
-
-    return _.find(letters, refId => {
-      return _.every(this.targets, other => {
-        return other.refId !== refId;
-      });
-    });
-  }
-
   changeQuery(query: DataQuery, index: number) {
     // ensure refId is maintained
     query.refId = this.targets[index].refId;

public/app/features/explore/state/actions.ts

@@ -60,7 +60,6 @@ import {
   splitCloseAction,
   splitOpenAction,
   addQueryRowAction,
-  AddQueryRowPayload,
   toggleGraphAction,
   toggleLogsAction,
   toggleTableAction,
@@ -87,9 +86,12 @@ const updateExploreUIState = (exploreId, uiStateFragment: Partial<ExploreUIState
 /**
  * Adds a query row after the row with the given index.
  */
-export function addQueryRow(exploreId: ExploreId, index: number): ActionOf<AddQueryRowPayload> {
+export function addQueryRow(exploreId: ExploreId, index: number): ThunkResult<void> {
-  const query = generateEmptyQuery(index + 1);
+  return (dispatch, getState) => {
-  return addQueryRowAction({ exploreId, index, query });
+    const query = generateEmptyQuery(getState().explore[exploreId].queries, index);
+
+    dispatch(addQueryRowAction({ exploreId, index, query }));
+  };
 }
 
 /**
@@ -126,10 +128,10 @@ export function changeQuery(
   index: number,
   override: boolean
 ): ThunkResult<void> {
-  return dispatch => {
+  return (dispatch, getState) => {
     // Null query means reset
     if (query === null) {
-      query = { ...generateEmptyQuery(index) };
+      query = { ...generateEmptyQuery(getState().explore[exploreId].queries) };
     }
 
     dispatch(changeQueryAction({ exploreId, query, index, override }));
@@ -287,7 +289,7 @@ export function importQueries(
 
     const nextQueries = importedQueries.map((q, i) => ({
       ...q,
-      ...generateEmptyQuery(i),
+      ...generateEmptyQuery(queries),
     }));
 
     dispatch(queriesImportedAction({ exploreId, queries: nextQueries }));
@@ -629,9 +631,9 @@ export function scanStart(exploreId: ExploreId, scanner: RangeScanner): ThunkRes
  * Use this action for clicks on query examples. Triggers a query run.
  */
 export function setQueries(exploreId: ExploreId, rawQueries: DataQuery[]): ThunkResult<void> {
-  return dispatch => {
+  return (dispatch, getState) => {
     // Inject react keys into query objects
-    const queries = rawQueries.map(q => ({ ...q, ...generateEmptyQuery() }));
+    const queries = rawQueries.map(q => ({ ...q, ...generateEmptyQuery(getState().explore[exploreId].queries) }));
     dispatch(setQueriesAction({ exploreId, queries }));
     dispatch(runQueries(exploreId));
   };

public/app/features/explore/state/reducers.ts

@@ -127,7 +127,7 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
       const { query, index } = action.payload;
 
       // Override path: queries are completely reset
-      const nextQuery: DataQuery = { ...query, ...generateEmptyQuery(index) };
+      const nextQuery: DataQuery = { ...query, ...generateEmptyQuery(state.queries) };
       const nextQueries = [...queries];
       nextQueries[index] = nextQuery;
 
@@ -267,7 +267,7 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
         // Modify all queries
         nextQueries = queries.map((query, i) => ({
           ...modifier({ ...query }, modification),
-          ...generateEmptyQuery(i),
+          ...generateEmptyQuery(state.queries),
         }));
         // Discard all ongoing transactions
         nextQueryTransactions = [];
@@ -276,7 +276,9 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
         nextQueries = queries.map((query, i) => {
           // Synchronize all queries with local query cache to ensure consistency
           // TODO still needed?
-          return i === index ? { ...modifier({ ...query }, modification), ...generateEmptyQuery(i) } : query;
+          return i === index
+            ? { ...modifier({ ...query }, modification), ...generateEmptyQuery(state.queries) }
+            : query;
         });
         nextQueryTransactions = queryTransactions
           // Consume the hint corresponding to the action