From ae164df69875046763ad35df7ec2a047901f23a2 Mon Sep 17 00:00:00 2001
From: Brendan O'Handley <brendan.ohandley@grafana.com>
Date: Mon, 20 Nov 2023 14:33:16 -0500
Subject: [PATCH] Prometheus: Fix UI bug where a label with empty string shows
 as populated with the deleted label filter value (#78153)

* preserve label with empty string

* fix side effect of having undefined show up in the editor select for value

* re render label item when label filters change
---
 .../components/LabelFilterItem.tsx             | 10 ++++++----
 .../components/LabelFilters.test.tsx           | 18 +++++++++++++++++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilterItem.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilterItem.tsx
index bdbe4795f9e..fdaa490f275 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilterItem.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilterItem.tsx
@@ -73,8 +73,10 @@ export function LabelFilterItem({
     debounceDuration
   );
 
+  const itemValue = item?.value ?? '';
+
   return (
-    <div data-testid="prometheus-dimensions-filter-item">
+    <div key={itemValue} data-testid="prometheus-dimensions-filter-item">
       <InputGroup>
         {/* Label name select, loads all values at once */}
         <Select
@@ -136,8 +138,8 @@ export function LabelFilterItem({
           width="auto"
           value={
             isMultiSelect()
-              ? getSelectOptionsFromString(item?.value).map(toOption)
-              : getSelectOptionsFromString(item?.value).map(toOption)[0]
+              ? getSelectOptionsFromString(itemValue).map(toOption)
+              : getSelectOptionsFromString(itemValue).map(toOption)[0]
           }
           allowCustomValue
           onOpenMenu={async () => {
@@ -179,7 +181,7 @@ export function LabelFilterItem({
           }}
           invalid={invalidValue}
         />
-        <AccessoryButton aria-label="remove" icon="times" variant="secondary" onClick={onDelete} />
+        <AccessoryButton aria-label={`remove-${item.label}`} icon="times" variant="secondary" onClick={onDelete} />
       </InputGroup>
     </div>
   );
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilters.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilters.test.tsx
index 19c9fb4cafc..71063d6f75d 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilters.test.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/components/LabelFilters.test.tsx
@@ -66,10 +66,26 @@ describe('LabelFilters', () => {
 
   it('removes label', async () => {
     const { onChange } = setup({ labelsFilters: [{ label: 'foo', op: '=', value: 'bar' }] });
-    await userEvent.click(screen.getByLabelText(/remove/));
+    await userEvent.click(screen.getByLabelText(/remove-foo/));
     expect(onChange).toBeCalledWith([]);
   });
 
+  it('removes label but preserves a label with a value of empty string', async () => {
+    const { onChange } = setup({
+      labelsFilters: [
+        { label: 'lab', op: '=', value: 'bel' },
+        { label: 'foo', op: '=', value: 'bar' },
+        { label: 'le', op: '=', value: '' },
+      ],
+    });
+    await userEvent.click(screen.getByLabelText(/remove-foo/));
+    expect(onChange).toBeCalledWith([
+      { label: 'lab', op: '=', value: 'bel' },
+      { label: 'le', op: '=', value: '' },
+    ]);
+    expect(screen.queryByText('bar')).toBeNull();
+  });
+
   it('renders empty input when labels are deleted from outside ', async () => {
     const { rerender } = setup({ labelsFilters: [{ label: 'foo', op: '=', value: 'bar' }] });
     expect(screen.getByText(/foo/)).toBeInTheDocument();