Merge pull request #109 from grafana/ivana/label-filters-bug

LabelFilters: Fix bug with running incomplete query
grafana · Jan 24, 2024 · 96a0221 · 96a0221
2 parents 56df7f7 + db4932e
commit 96a0221
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 6 deletions.
diff --git a/src/VisualQueryBuilder/components/LabelFilterItem.tsx b/src/VisualQueryBuilder/components/LabelFilterItem.tsx
@@ -14,7 +14,7 @@ interface Props {
   defaultOp: string;
   item: Partial<QueryBuilderLabelFilter>;
   items: Array<Partial<QueryBuilderLabelFilter>>;
-  onChange: (value: QueryBuilderLabelFilter) => void;
+  onChange: (value: Partial<QueryBuilderLabelFilter>) => void;
   onGetLabelNames: (forLabel: Partial<QueryBuilderLabelFilter>) => Promise<Array<SelectableValue<string>>>;
   onGetLabelValues: (forLabel: Partial<QueryBuilderLabelFilter>) => Promise<Array<SelectableValue<string>>>;
   onDelete: () => void;
@@ -98,7 +98,6 @@ export function LabelFilterItem({
               if (change.value) {
                 onChange({
                   ...item,
-                  value: item.value ?? '',
                   op: item.op ?? defaultOp,
                   label: change.value,
                 });
@@ -116,9 +115,8 @@ export function LabelFilterItem({
               if (change.value) {
                 onChange({
                   ...item,
-                  label: item.label ?? '',
                   op: change.value,
-                  value: isMultiSelect(change.value) ? (item.value ?? '') : getSelectOptionsFromString(item?.value)[0],
+                  value: isMultiSelect(change.value) ? item.value  : getSelectOptionsFromString(item?.value)[0],
                 });
               }
             }}
@@ -159,7 +157,6 @@ export function LabelFilterItem({
                   ...item,
                   value: change.value,
                   op: item.op ?? defaultOp,
-                  label: item.label ?? '',
                 });
               } else {
                 // otherwise, we're dealing with a multi-value select which is array of options
@@ -173,7 +170,7 @@ export function LabelFilterItem({
                   })
                   .filter((val: string | undefined) => val !== undefined)
                   .join(multiValueSeparator);
-                onChange({ ...item, label: item.label ?? '', value: changes, op: item.op ?? defaultOp });
+                onChange({ ...item, value: changes, op: item.op ?? defaultOp });
               }
             }}
             invalid={isConflicting || invalidValue}

diff --git a/src/VisualQueryBuilder/components/LabelFilters.test.tsx b/src/VisualQueryBuilder/components/LabelFilters.test.tsx
@@ -89,6 +89,23 @@ describe('LabelFilters', () => {
     setup({ labelsFilters: [], labelFilterRequired: true });
     expect(screen.getByText(MISSING_LABEL_FILTER_ERROR_MESSAGE)).toBeInTheDocument();
   });
+
+  it('runs onChange after all selects are changed', async () => {
+    const { onChange } = setup({ labelsFilters: [{ label: 'foo', op: '=', value: 'bar' }] });
+    await userEvent.click(getAddButton());
+    expect(screen.getAllByText('Select label')).toHaveLength(1);
+    expect(screen.getAllByText('Select value')).toHaveLength(1);
+    const { name, value, op } = getLabelSelects(1);
+    await selectOptionInTest(name, 'baz');
+    expect(onChange).not.toBeCalled();
+    await selectOptionInTest(op, '!=');
+    expect(onChange).not.toBeCalled();
+    await selectOptionInTest(value, 'qux');
+    expect(onChange).toBeCalledWith([
+      { label: "foo", op: "=", value: "bar" }, 
+      { label: "baz", op: "!=", value: "qux" }
+    ]);    
+  });
 });
 
 function setup(propOverrides?: Partial<ComponentProps<typeof LabelFilters>>) {
@@ -122,6 +139,7 @@ function getLabelSelects(index = 0) {
   const selects = getAllByRole(labels.parentElement!.parentElement!.parentElement!, 'combobox');
   return {
     name: selects[3 * index],
+    op: selects[3 * index + 1],
     value: selects[3 * index + 2],
   };
 }