From 9310bb632e149d324e22d3eba1d6a43807f83a13 Mon Sep 17 00:00:00 2001
From: Matias Chomicki <matyax@gmail.com>
Date: Wed, 6 Sep 2023 10:25:53 +0200
Subject: [PATCH] Loki Query Builder: ensure unique ids for labelled fields
 (#74398)

* Loki Query Builder: ensure unique ids for labelled fields

* Rename refactored argument
---
 .../querybuilder/components/UnwrapParamEditor.test.tsx   | 2 +-
 .../loki/querybuilder/components/UnwrapParamEditor.tsx   | 4 ++--
 .../querybuilder/components/LabelParamEditor.tsx         | 4 ++--
 .../components/PromQueryBuilderContainer.test.tsx        | 2 +-
 .../prometheus/querybuilder/shared/OperationEditor.tsx   | 7 ++++---
 .../querybuilder/shared/OperationParamEditor.tsx         | 8 ++++----
 .../querybuilder/shared/operationUtils.test.ts           | 9 +++++++++
 .../prometheus/querybuilder/shared/operationUtils.ts     | 4 ++--
 .../datasource/prometheus/querybuilder/shared/types.ts   | 2 +-
 9 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.test.tsx b/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.test.tsx
index 59bb8e1ad4c..a0b7803a500 100644
--- a/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.test.tsx
+++ b/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.test.tsx
@@ -63,7 +63,7 @@ const createProps = (
     onChange: jest.fn(),
     onRunQuery: jest.fn(),
     index: 1,
-    operationIndex: 1,
+    operationId: '1',
     query: {
       labels: [{ op: '=', label: 'foo', value: 'bar' }],
       operations: [
diff --git a/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.tsx b/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.tsx
index 99852a01640..4e364fc8290 100644
--- a/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.tsx
+++ b/public/app/plugins/datasource/loki/querybuilder/components/UnwrapParamEditor.tsx
@@ -15,7 +15,7 @@ import { LokiVisualQuery } from '../types';
 export function UnwrapParamEditor({
   onChange,
   index,
-  operationIndex,
+  operationId,
   value,
   query,
   datasource,
@@ -27,7 +27,7 @@ export function UnwrapParamEditor({
 
   return (
     <Select
-      inputId={getOperationParamId(operationIndex, index)}
+      inputId={getOperationParamId(operationId, index)}
       onOpenMenu={async () => {
         // This check is always true, we do it to make typescript happy
         if (datasource instanceof LokiDatasource) {
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/LabelParamEditor.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/LabelParamEditor.tsx
index 941bb1e11f3..bca29531682 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/components/LabelParamEditor.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/components/LabelParamEditor.tsx
@@ -11,7 +11,7 @@ import { PromVisualQuery } from '../types';
 export function LabelParamEditor({
   onChange,
   index,
-  operationIndex,
+  operationId,
   value,
   query,
   datasource,
@@ -23,7 +23,7 @@ export function LabelParamEditor({
 
   return (
     <Select
-      inputId={getOperationParamId(operationIndex, index)}
+      inputId={getOperationParamId(operationId, index)}
       autoFocus={value === '' ? true : undefined}
       openMenuOnFocus
       onOpenMenu={async () => {
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx
index 2ad87998957..3816fb9a3ec 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderContainer.test.tsx
@@ -30,7 +30,7 @@ describe('PromQueryBuilderContainer', () => {
     await userEvent.click(screen.getByTestId('operations.0.add-rest-param'));
 
     waitFor(() => {
-      expect(container.querySelector(`${getOperationParamId(0, 0)}`)).toBeInTheDocument();
+      expect(container.querySelector(`${getOperationParamId('0', 0)}`)).toBeInTheDocument();
     });
   });
 });
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationEditor.tsx b/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationEditor.tsx
index 12f59778029..1b3370b679c 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationEditor.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationEditor.tsx
@@ -1,5 +1,5 @@
 import { css, cx } from '@emotion/css';
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useId, useState } from 'react';
 import { Draggable } from 'react-beautiful-dnd';
 
 import { DataSourceApi, GrafanaTheme2 } from '@grafana/data';
@@ -46,6 +46,7 @@ export function OperationEditor({
 }: Props) {
   const def = queryModeller.getOperationDef(operation.id);
   const shouldFlash = useFlash(flash);
+  const id = useId();
 
   const isConflicting =
     operation.id === LokiOperationId.LabelFilter && isConflictingFilter(operation, query.operations);
@@ -86,7 +87,7 @@ export function OperationEditor({
       <div className={styles.paramRow} key={`${paramIndex}-1`}>
         {!paramDef.hideName && (
           <div className={styles.paramName}>
-            <label htmlFor={getOperationParamId(index, paramIndex)}>{paramDef.name}</label>
+            <label htmlFor={getOperationParamId(id, paramIndex)}>{paramDef.name}</label>
             {paramDef.description && (
               <Tooltip placement="top" content={paramDef.description} theme="info">
                 <Icon name="info-circle" size="sm" className={styles.infoIcon} />
@@ -101,7 +102,7 @@ export function OperationEditor({
               paramDef={paramDef}
               value={operation.params[paramIndex]}
               operation={operation}
-              operationIndex={index}
+              operationId={id}
               onChange={onParamValueChanged}
               onRunQuery={onRunQuery}
               query={query}
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationParamEditor.tsx b/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationParamEditor.tsx
index de069eb71ff..e0e8673670b 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationParamEditor.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/OperationParamEditor.tsx
@@ -33,7 +33,7 @@ export function getOperationParamEditor(
 function SimpleInputParamEditor(props: QueryBuilderOperationParamEditorProps) {
   return (
     <AutoSizeInput
-      id={getOperationParamId(props.operationIndex, props.index)}
+      id={getOperationParamId(props.operationId, props.index)}
       defaultValue={props.value?.toString()}
       minWidth={props.paramDef.minWidth}
       placeholder={props.paramDef.placeholder}
@@ -52,7 +52,7 @@ function SimpleInputParamEditor(props: QueryBuilderOperationParamEditorProps) {
 function BoolInputParamEditor(props: QueryBuilderOperationParamEditorProps) {
   return (
     <Checkbox
-      id={getOperationParamId(props.operationIndex, props.index)}
+      id={getOperationParamId(props.operationId, props.index)}
       value={props.value as boolean}
       onChange={(evt) => props.onChange(props.index, evt.currentTarget.checked)}
     />
@@ -63,7 +63,7 @@ function SelectInputParamEditor({
   paramDef,
   value,
   index,
-  operationIndex,
+  operationId,
   onChange,
 }: QueryBuilderOperationParamEditorProps) {
   const styles = useStyles2(getStyles);
@@ -99,7 +99,7 @@ function SelectInputParamEditor({
   return (
     <Stack gap={0.5} direction="row" alignItems="center" wrap={false}>
       <Select
-        id={getOperationParamId(operationIndex, index)}
+        id={getOperationParamId(operationId, index)}
         value={valueOption}
         options={selectOptions}
         placeholder={paramDef.placeholder}
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.test.ts b/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.test.ts
index 03d45ca8e52..da1aabe924e 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.test.ts
+++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.test.ts
@@ -1,6 +1,7 @@
 import {
   createAggregationOperation,
   createAggregationOperationWithParam,
+  getOperationParamId,
   isConflictingSelector,
 } from './operationUtils';
 
@@ -197,3 +198,11 @@ describe('isConflictingSelector', () => {
     expect(isConflictingSelector(newLabel, labels)).toBe(false);
   });
 });
+
+describe('getOperationParamId', () => {
+  it('Generates correct id for operation param', () => {
+    const operationId = 'abc';
+    const paramId = 0;
+    expect(getOperationParamId(operationId, paramId)).toBe('operations.abc.param.0');
+  });
+});
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.ts b/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.ts
index cdd7988b2ea..eb09cb98896 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.ts
+++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/operationUtils.ts
@@ -120,8 +120,8 @@ export function getPromAndLokiOperationDisplayName(funcName: string) {
   return capitalize(funcName.replace(/_/g, ' '));
 }
 
-export function getOperationParamId(operationIndex: number, paramIndex: number) {
-  return `operations.${operationIndex}.param.${paramIndex}`;
+export function getOperationParamId(operationId: string, paramIndex: number) {
+  return `operations.${operationId}.param.${paramIndex}`;
 }
 
 export function getRangeVectorParamDef(withRateInterval = false): QueryBuilderOperationParamDef {
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts b/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts
index 5f3a3f57b2f..7655a6fb279 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts
+++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/types.ts
@@ -89,7 +89,7 @@ export interface QueryBuilderOperationParamEditorProps {
   /** Parameter index */
   index: number;
   operation: QueryBuilderOperation;
-  operationIndex: number;
+  operationId: string;
   query: any;
   datasource: DataSourceApi;
   onChange: (index: number, value: QueryBuilderOperationParamValue) => void;