From 2ed5fd9adee317be759a9dec8e326f56b489567b Mon Sep 17 00:00:00 2001
From: Marta Bondyra <4283304+mbondyra@users.noreply.github.com>
Date: Tue, 30 Jul 2024 16:44:54 +0200
Subject: [PATCH] [Lens] do not pass incorrect filters to the state (#189292)

## Summary

Before we would pass incorrect filters to the state and save it and then
in the renderer filter whatever we couldn't process.
That unfortunately lead to the bug in another piece of code - when
creating a link to explore underlying data in discover.
This code is aligned with how we do filtering for filtered metric - we
don't pass the filter to the state, unless it is valid.
---
 .../filters/filter_popover.test.tsx           | 29 ++++++++++++-----
 .../definitions/filters/filter_popover.tsx    | 31 +++++++++++++------
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.test.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.test.tsx
index 8e7fb9e310f664..73842e6cae114e 100644
--- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.test.tsx
+++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.test.tsx
@@ -14,16 +14,21 @@ import { FilterPopover } from './filter_popover';
 import { LabelInput } from '../shared_components';
 import { QueryStringInput } from '@kbn/unified-search-plugin/public';
 import { QueryInput } from '@kbn/visualization-ui-components';
+import { Query } from '@kbn/es-query';
 
-jest.mock('.', () => ({
-  isQueryValid: () => true,
-  defaultLabel: 'label',
-}));
+jest.mock('.', () => ({}));
+
+jest.mock('@kbn/visualization-ui-components', () => {
+  const original = jest.requireActual('@kbn/visualization-ui-components');
+
+  return {
+    ...original,
+    isQueryValid: jest.fn((q: Query) => (q.query === 'bytes >= 1 and' ? false : true)),
+  };
+});
 
 jest.mock('@kbn/unified-search-plugin/public', () => ({
-  QueryStringInput: () => {
-    return 'QueryStringInput';
-  },
+  QueryStringInput: () => 'QueryStringInput',
 }));
 
 describe('filter popover', () => {
@@ -118,6 +123,16 @@ describe('filter popover', () => {
     });
   });
 
+  it('should not call setFilter if QueryInput value is not valid', () => {
+    const setFilter = jest.fn();
+    const instance = shallow(<FilterPopover {...defaultProps} setFilter={setFilter} />);
+    instance.find(QueryInput).prop('onChange')!({
+      query: 'bytes >= 1 and',
+      language: 'kuery',
+    });
+    expect(setFilter).not.toHaveBeenCalled();
+  });
+
   it('should call setFilter when modifying LabelInput', () => {
     const setFilter = jest.fn();
     const instance = shallow(<FilterPopover {...defaultProps} setFilter={setFilter} />);
diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.tsx
index 35327fb91b6781..1f3f5ba94e63bc 100644
--- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.tsx
+++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/filters/filter_popover.tsx
@@ -7,7 +7,7 @@
 
 import './filter_popover.scss';
 
-import React from 'react';
+import React, { useState } from 'react';
 import { EuiPopover, EuiSpacer } from '@elastic/eui';
 import type { Query } from '@kbn/es-query';
 // Need to keep it separate to make it work Jest mocks in dimension_panel tests
@@ -36,9 +36,18 @@ export const FilterPopover = ({
   triggerClose: () => void;
 }) => {
   const inputRef = React.useRef<HTMLInputElement>();
+  const [localFilter, setLocalFilter] = useState(() => filter);
 
-  const setFilterLabel = (label: string) => setFilter({ ...filter, label });
-  const setFilterQuery = (input: Query) => setFilter({ ...filter, input });
+  const setFilterLabel = (label: string) => {
+    setLocalFilter({ ...localFilter, label });
+    setFilter({ ...filter, label });
+  };
+  const setFilterQuery = (input: Query) => {
+    setLocalFilter({ ...localFilter, input });
+    if (isQueryValid(input, indexPattern)) {
+      setFilter({ ...filter, input });
+    }
+  };
 
   const getPlaceholder = (query: Query['query']) => {
     if (query === '') {
@@ -49,6 +58,10 @@ export const FilterPopover = ({
       return String(query);
     }
   };
+  const closePopover = () => {
+    setLocalFilter({ ...localFilter, input: filter.input });
+    triggerClose();
+  };
 
   return (
     <EuiPopover
@@ -56,12 +69,12 @@ export const FilterPopover = ({
       panelClassName="lnsIndexPatternDimensionEditor__filtersEditor"
       isOpen={isOpen}
       ownFocus
-      closePopover={triggerClose}
+      closePopover={closePopover}
       button={button}
     >
       <QueryInput
-        isInvalid={!isQueryValid(filter.input, indexPattern)}
-        value={filter.input}
+        isInvalid={!isQueryValid(localFilter.input, indexPattern)}
+        value={localFilter.input}
         dataView={
           indexPattern.id
             ? { type: 'id', value: indexPattern.id }
@@ -77,11 +90,11 @@ export const FilterPopover = ({
       />
       <EuiSpacer size="s" />
       <LabelInput
-        value={filter.label || ''}
+        value={localFilter.label || ''}
         onChange={setFilterLabel}
-        placeholder={getPlaceholder(filter.input.query)}
+        placeholder={getPlaceholder(localFilter.input.query)}
         inputRef={inputRef}
-        onSubmit={triggerClose}
+        onSubmit={closePopover}
         dataTestSubj="indexPattern-filters-label"
       />
     </EuiPopover>