diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx index 1eeb939edbe..2848d885736 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx @@ -38,10 +38,9 @@ import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetr import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; import fetchMock from 'fetch-mock'; -import { TestDataset } from '@superset-ui/chart-controls'; +import { TestDataset, Dataset } from '@superset-ui/chart-controls'; import AdhocFilterEditPopoverSimpleTabContent, { useSimpleTabFilterProps, - Props, } from '.'; import { Clauses, ExpressionTypes } from '../types'; @@ -144,228 +143,11 @@ jest.mock('@superset-ui/core', () => ({ const mockedIsFeatureEnabled = isFeatureEnabled as jest.Mock; -// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks -describe('AdhocFilterEditPopoverSimpleTabContent', () => { - test('can render the simple tab form', () => { - expect(() => setup()).not.toThrow(); - }); - - test('shows boolean only operators when subject is boolean', () => { - const props = setup({ - adhocFilter: new AdhocFilter({ - expressionType: ExpressionTypes.Simple, - subject: 'value', - operatorId: null, - operator: null, - comparator: null, - clause: null, - }), - datasource: { - columns: [ - { - id: 3, - column_name: 'value', - type: 'BOOL', - }, - ], - }, - }); - const { isOperatorRelevant } = useSimpleTabFilterProps(props); - [ - Operators.IsTrue, - Operators.IsFalse, - Operators.IsNull, - Operators.IsFalse, - ].map(operator => expect(isOperatorRelevant(operator, 'value')).toBe(true)); - }); - test('shows boolean only operators when subject is number', () => { - const props = setup({ - adhocFilter: new AdhocFilter({ - expressionType: ExpressionTypes.Simple, - subject: 'value', - operatorId: null, - operator: null, - comparator: null, - clause: null, - }), - datasource: { - columns: [ - { - id: 3, - column_name: 'value', - type: 'INT', - }, - ], - }, - }); - const { isOperatorRelevant } = useSimpleTabFilterProps(props); - [ - Operators.IsTrue, - Operators.IsFalse, - Operators.IsNull, - Operators.IsNotNull, - ].map(operator => expect(isOperatorRelevant(operator, 'value')).toBe(true)); - }); - - test('will convert from individual comparator to array if the operator changes to multi', () => { - const props = setup(); - const { onOperatorChange } = useSimpleTabFilterProps(props); - onOperatorChange(Operators.In); - expect(props.onChange.calledOnce).toBe(true); - expect(props.onChange.lastCall.args[0].comparator).toEqual(['10']); - expect(props.onChange.lastCall.args[0].operatorId).toEqual(Operators.In); - }); - - test('will convert from array to individual comparators if the operator changes from multi', () => { - const props = setup({ - adhocFilter: simpleMultiAdhocFilter, - }); - const { onOperatorChange } = useSimpleTabFilterProps(props); - onOperatorChange(Operators.LessThan); - expect(props.onChange.calledOnce).toBe(true); - expect(props.onChange.lastCall.args[0]).toEqual( - simpleMultiAdhocFilter.duplicateWith({ - operatorId: Operators.LessThan, - operator: '<', - comparator: '10', - }), - ); - }); - - test('passes the new adhocFilter to onChange after onComparatorChange', () => { - const props = setup(); - const { onComparatorChange } = useSimpleTabFilterProps(props); - onComparatorChange('20'); - expect(props.onChange.calledOnce).toBe(true); - expect(props.onChange.lastCall.args[0]).toEqual( - simpleAdhocFilter.duplicateWith({ comparator: '20' }), - ); - }); - - test('will filter operators for table datasources', () => { - const props = setup({ datasource: { type: 'table' } }); - const { isOperatorRelevant } = useSimpleTabFilterProps(props); - expect(isOperatorRelevant(Operators.Like, 'value')).toBe(true); - }); - - test('will show LATEST PARTITION operator', () => { - const props = setup({ - datasource: { - type: 'table', - datasource_name: 'table1', - schema: 'schema', - }, - adhocFilter: simpleCustomFilter, - partitionColumn: 'ds', - }); - const { isOperatorRelevant } = useSimpleTabFilterProps(props); - expect(isOperatorRelevant(Operators.LatestPartition, 'ds')).toBe(true); - expect(isOperatorRelevant(Operators.LatestPartition, 'value')).toBe(false); - }); - - test('will generate custom sqlExpression for LATEST PARTITION operator', () => { - const testAdhocFilter = new AdhocFilter({ - expressionType: ExpressionTypes.Simple, - subject: 'ds', - }); - const props = setup({ - datasource: { - type: 'table', - datasource_name: 'table1', - schema: 'schema', - }, - adhocFilter: testAdhocFilter, - partitionColumn: 'ds', - }); - const { onOperatorChange } = useSimpleTabFilterProps(props); - onOperatorChange(Operators.LatestPartition); - expect(props.onChange.calledOnce).toBe(true); - expect(props.onChange.lastCall.args[0]).toEqual( - testAdhocFilter.duplicateWith({ - subject: 'ds', - operator: 'LATEST PARTITION', - operatorId: Operators.LatestPartition, - comparator: null, - clause: 'WHERE', - expressionType: 'SQL', - sqlExpression: "ds = '{{ presto.latest_partition('schema.table1') }}'", - }), - ); - }); - test('will not display boolean operators when column type is string', () => { - const props = setup({ - datasource: { - type: 'table', - datasource_name: 'table1', - schema: 'schema', - columns: [{ column_name: 'value', type: 'STRING' }], - }, - adhocFilter: simpleAdhocFilter, - }); - const { isOperatorRelevant } = useSimpleTabFilterProps(props); - const booleanOnlyOperators = [Operators.IsTrue, Operators.IsFalse]; - booleanOnlyOperators.forEach(operator => { - expect(isOperatorRelevant(operator, 'value')).toBe(false); - }); - }); - test('will display boolean operators when column is an expression', () => { - const props = setup({ - datasource: { - type: 'table', - datasource_name: 'table1', - schema: 'schema', - columns: [ - { - column_name: 'value', - expression: 'case when value is 0 then "NO"', - }, - ], - }, - adhocFilter: simpleAdhocFilter, - }); - const { isOperatorRelevant } = useSimpleTabFilterProps(props); - const booleanOnlyOperators = [Operators.IsTrue, Operators.IsFalse]; - booleanOnlyOperators.forEach(operator => { - expect(isOperatorRelevant(operator, 'value')).toBe(true); - }); - }); - test('sets comparator to undefined when operator is IS_TRUE', () => { - const props = setup(); - const { onOperatorChange } = useSimpleTabFilterProps(props); - onOperatorChange(Operators.IsTrue); - expect(props.onChange.calledOnce).toBe(true); - expect(props.onChange.lastCall.args[0].operatorId).toBe(Operators.IsTrue); - expect(props.onChange.lastCall.args[0].operator).toBe('IS TRUE'); - expect(props.onChange.lastCall.args[0].comparator).toBe(undefined); - }); - test('sets comparator to undefined when operator is IS_FALSE', () => { - const props = setup(); - const { onOperatorChange } = useSimpleTabFilterProps(props); - onOperatorChange(Operators.IsFalse); - expect(props.onChange.calledOnce).toBe(true); - expect(props.onChange.lastCall.args[0].operatorId).toBe(Operators.IsFalse); - expect(props.onChange.lastCall.args[0].operator).toBe('IS FALSE'); - expect(props.onChange.lastCall.args[0].comparator).toBe(undefined); - }); - test('sets comparator to undefined when operator is IS_NULL or IS_NOT_NULL', () => { - const props = setup(); - const { onOperatorChange } = useSimpleTabFilterProps(props); - [Operators.IsNull, Operators.IsNotNull].forEach(op => { - onOperatorChange(op); - expect(props.onChange.called).toBe(true); - expect(props.onChange.lastCall.args[0].operatorId).toBe(op); - expect(props.onChange.lastCall.args[0].operator).toBe( - OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation, - ); - expect(props.onChange.lastCall.args[0].comparator).toBe(undefined); - }); - }); -}); - const ADVANCED_DATA_TYPE_ENDPOINT_VALID = 'glob:*/api/v1/advanced_data_type/convert?q=(type:type,values:!(v))'; const ADVANCED_DATA_TYPE_ENDPOINT_INVALID = 'glob:*/api/v1/advanced_data_type/convert?q=(type:type,values:!(e))'; + fetchMock.get(ADVANCED_DATA_TYPE_ENDPOINT_VALID, { result: { display_value: 'VALID', @@ -382,182 +164,436 @@ fetchMock.get(ADVANCED_DATA_TYPE_ENDPOINT_INVALID, { values: [], }, }); + const mockStore = configureStore([thunk]); const store = mockStore({}); -// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks -describe('AdhocFilterEditPopoverSimpleTabContent Advanced data Type Test', () => { - const setupFilter = async (props: Props) => { - await act(async () => { - render(, { - store, - }); - }); - }; +let isFeatureEnabledMock: jest.SpyInstance; - let isFeatureEnabledMock: any; - beforeEach(async () => { - isFeatureEnabledMock = mockedIsFeatureEnabled.mockImplementation( - (featureFlag: FeatureFlag) => - featureFlag === FeatureFlag.EnableAdvancedDataTypes, - ); - }); +beforeEach(() => { + fetchMock.resetHistory(); + isFeatureEnabledMock = mockedIsFeatureEnabled.mockImplementation( + (featureFlag: FeatureFlag) => + featureFlag === FeatureFlag.EnableAdvancedDataTypes, + ); +}); - afterAll(() => { +afterAll(() => { + if (isFeatureEnabledMock) { isFeatureEnabledMock.mockRestore(); - }); + } +}); - test('should not call API when column has no advanced data type', async () => { - fetchMock.resetHistory(); +test('can render the simple tab form', () => { + expect(() => setup()).not.toThrow(); +}); - const props = getAdvancedDataTypeTestProps(); - - await setupFilter(props); - - const filterValueField = screen.getByPlaceholderText( - 'Filter value (case sensitive)', - ); - await act(async () => { - userEvent.type(filterValueField, 'v'); - }); - - await act(async () => { - userEvent.type(filterValueField, '{enter}'); - }); - - // When the column is not a advanced data type, - // the advanced data type endpoint should not be called - await waitFor(() => - expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_VALID)).toHaveLength( - 0, - ), - ); - }); - - test('should call API when column has advanced data type', async () => { - fetchMock.resetHistory(); - - const props = getAdvancedDataTypeTestProps({ - options: [ +test('shows boolean only operators when subject is boolean', () => { + const props = setup({ + adhocFilter: new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'value', + operatorId: null, + operator: null, + comparator: null, + clause: null, + }), + datasource: { + columns: [ { - type: 'DOUBLE', - column_name: 'advancedDataType', - id: 5, - advanced_data_type: 'type', + id: 3, + column_name: 'value', + type: 'BOOL', }, ], - }); - - await setupFilter(props); - - const filterValueField = screen.getByPlaceholderText( - 'Filter value (case sensitive)', - ); - await act(async () => { - userEvent.type(filterValueField, 'v'); - }); - - await act(async () => { - userEvent.type(filterValueField, '{enter}'); - }); - - // When the column is a advanced data type, - // the advanced data type endpoint should be called - await waitFor(() => - expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_VALID)).toHaveLength( - 1, - ), - ); - expect(props.validHandler.lastCall.args[0]).toBe(true); + }, }); + const { isOperatorRelevant } = useSimpleTabFilterProps(props); + [ + Operators.IsTrue, + Operators.IsFalse, + Operators.IsNull, + Operators.IsFalse, + ].map(operator => expect(isOperatorRelevant(operator, 'value')).toBe(true)); +}); - test('save button should be disabled if error message from API is returned', async () => { - fetchMock.resetHistory(); - - const props = getAdvancedDataTypeTestProps({ - options: [ +test('shows boolean only operators when subject is number', () => { + const props = setup({ + adhocFilter: new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'value', + operatorId: null, + operator: null, + comparator: null, + clause: null, + }), + datasource: { + columns: [ { - type: 'DOUBLE', - column_name: 'advancedDataType', - id: 5, - advanced_data_type: 'type', + id: 3, + column_name: 'value', + type: 'INT', }, ], - }); - - await setupFilter(props); - - const filterValueField = screen.getByPlaceholderText( - 'Filter value (case sensitive)', - ); - await act(async () => { - userEvent.type(filterValueField, 'e'); - }); - - await act(async () => { - userEvent.type(filterValueField, '{enter}'); - }); - - // When the column is a advanced data type but an error response is given by the endpoint, - // the save button should be disabled - await waitFor(() => - expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_INVALID)).toHaveLength( - 1, - ), - ); - expect(props.validHandler.lastCall.args[0]).toBe(false); + }, }); + const { isOperatorRelevant } = useSimpleTabFilterProps(props); + [ + Operators.IsTrue, + Operators.IsFalse, + Operators.IsNull, + Operators.IsNotNull, + ].map(operator => expect(isOperatorRelevant(operator, 'value')).toBe(true)); +}); - test('advanced data type operator list should update after API response', async () => { - fetchMock.resetHistory(); +test('will convert from individual comparator to array if the operator changes to multi', () => { + const props = setup(); + const { onOperatorChange } = useSimpleTabFilterProps(props); + onOperatorChange(Operators.In); + expect(props.onChange.calledOnce).toBe(true); + expect(props.onChange.lastCall.args[0].comparator).toEqual(['10']); + expect(props.onChange.lastCall.args[0].operatorId).toEqual(Operators.In); +}); - const props = getAdvancedDataTypeTestProps({ - options: [ - { - type: 'DOUBLE', - column_name: 'advancedDataType', - id: 5, - advanced_data_type: 'type', - }, - ], - }); +test('will convert from array to individual comparators if the operator changes from multi', () => { + const props = setup({ + adhocFilter: simpleMultiAdhocFilter, + }); + const { onOperatorChange } = useSimpleTabFilterProps(props); + onOperatorChange(Operators.LessThan); + expect(props.onChange.calledOnce).toBe(true); + expect(props.onChange.lastCall.args[0]).toEqual( + simpleMultiAdhocFilter.duplicateWith({ + operatorId: Operators.LessThan, + operator: '<', + comparator: '10', + }), + ); +}); - await setupFilter(props); +test('passes the new adhocFilter to onChange after onComparatorChange', () => { + const props = setup(); + const { onComparatorChange } = useSimpleTabFilterProps(props); + onComparatorChange('20'); + expect(props.onChange.calledOnce).toBe(true); + expect(props.onChange.lastCall.args[0]).toEqual( + simpleAdhocFilter.duplicateWith({ comparator: '20' }), + ); +}); - const filterValueField = screen.getByPlaceholderText( - 'Filter value (case sensitive)', - ); - await act(async () => { - userEvent.type(filterValueField, 'v'); - }); +test('will filter operators for table datasources', () => { + const props = setup({ datasource: { type: 'table' as const } }); + const { isOperatorRelevant } = useSimpleTabFilterProps(props); + expect(isOperatorRelevant(Operators.Like, 'value')).toBe(true); +}); - await act(async () => { - userEvent.type(filterValueField, '{enter}'); - }); +test('will show LATEST PARTITION operator', () => { + const props = setup({ + datasource: { + type: 'table' as const, + datasource_name: 'table1', + schema: 'schema', + }, + adhocFilter: simpleCustomFilter, + partitionColumn: 'ds', + }); + const { isOperatorRelevant } = useSimpleTabFilterProps(props); + expect(isOperatorRelevant(Operators.LatestPartition, 'ds')).toBe(true); + expect(isOperatorRelevant(Operators.LatestPartition, 'value')).toBe(false); +}); - // When the column is a advanced data type, - // the advanced data type endpoint should be called - await waitFor(() => - expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_VALID)).toHaveLength( - 1, - ), - ); - expect(props.validHandler.lastCall.args[0]).toBe(true); +test('will generate custom sqlExpression for LATEST PARTITION operator', () => { + const testAdhocFilter = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'ds', + }); + const props = setup({ + datasource: { + type: 'table' as const, + datasource_name: 'table1', + schema: 'schema', + }, + adhocFilter: testAdhocFilter, + partitionColumn: 'ds', + }); + const { onOperatorChange } = useSimpleTabFilterProps(props); + onOperatorChange(Operators.LatestPartition); + expect(props.onChange.calledOnce).toBe(true); + expect(props.onChange.lastCall.args[0]).toEqual( + testAdhocFilter.duplicateWith({ + subject: 'ds', + operator: 'LATEST PARTITION', + operatorId: Operators.LatestPartition, + comparator: null, + clause: 'WHERE', + expressionType: 'SQL', + sqlExpression: "ds = '{{ presto.latest_partition('schema.table1') }}'", + }), + ); +}); - const operatorValueField = screen.getByRole('combobox', { - name: 'Select operator', - }); - - userEvent.click(operatorValueField); - - await act(async () => { - userEvent.type(operatorValueField, '{enter}'); - }); - - expect( - await screen.findByText('Equal to (=)', { - selector: '.ant-select-selection-item', - }), - ).toBeInTheDocument(); +test('will not display boolean operators when column type is string', () => { + const props = setup({ + datasource: { + type: 'table' as const, + datasource_name: 'table1', + schema: 'schema', + columns: [{ column_name: 'value', type: 'STRING' }], + }, + adhocFilter: simpleAdhocFilter, + }); + const { isOperatorRelevant } = useSimpleTabFilterProps(props); + const booleanOnlyOperators = [Operators.IsTrue, Operators.IsFalse]; + booleanOnlyOperators.forEach(operator => { + expect(isOperatorRelevant(operator, 'value')).toBe(false); }); }); + +test('will display boolean operators when column is an expression', () => { + const props = setup({ + datasource: { + type: 'table' as const, + datasource_name: 'table1', + schema: 'schema', + columns: [ + { + column_name: 'value', + expression: 'case when value is 0 then "NO"', + }, + ], + }, + adhocFilter: simpleAdhocFilter, + }); + const { isOperatorRelevant } = useSimpleTabFilterProps(props); + const booleanOnlyOperators = [Operators.IsTrue, Operators.IsFalse]; + booleanOnlyOperators.forEach(operator => { + expect(isOperatorRelevant(operator, 'value')).toBe(true); + }); +}); + +test('sets comparator to undefined when operator is IS_TRUE', () => { + const props = setup(); + const { onOperatorChange } = useSimpleTabFilterProps(props); + onOperatorChange(Operators.IsTrue); + expect(props.onChange.calledOnce).toBe(true); + expect(props.onChange.lastCall.args[0].operatorId).toBe(Operators.IsTrue); + expect(props.onChange.lastCall.args[0].operator).toBe('IS TRUE'); + expect(props.onChange.lastCall.args[0].comparator).toBe(undefined); +}); + +test('sets comparator to undefined when operator is IS_FALSE', () => { + const props = setup(); + const { onOperatorChange } = useSimpleTabFilterProps(props); + onOperatorChange(Operators.IsFalse); + expect(props.onChange.calledOnce).toBe(true); + expect(props.onChange.lastCall.args[0].operatorId).toBe(Operators.IsFalse); + expect(props.onChange.lastCall.args[0].operator).toBe('IS FALSE'); + expect(props.onChange.lastCall.args[0].comparator).toBe(undefined); +}); + +test('sets comparator to undefined when operator is IS_NULL or IS_NOT_NULL', () => { + const props = setup(); + const { onOperatorChange } = useSimpleTabFilterProps(props); + [Operators.IsNull, Operators.IsNotNull].forEach(op => { + onOperatorChange(op); + expect(props.onChange.called).toBe(true); + expect(props.onChange.lastCall.args[0].operatorId).toBe(op); + expect(props.onChange.lastCall.args[0].operator).toBe( + OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation, + ); + expect(props.onChange.lastCall.args[0].comparator).toBe(undefined); + }); +}); + +test('should not call API when column has no advanced data type', async () => { + const props = getAdvancedDataTypeTestProps(); + + await act(async () => { + render(, { + store, + }); + }); + + const filterValueField = screen.getByPlaceholderText( + 'Filter value (case sensitive)', + ); + await act(async () => { + userEvent.type(filterValueField, 'v'); + }); + + await act(async () => { + userEvent.type(filterValueField, '{enter}'); + }); + + await waitFor(() => + expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_VALID)).toHaveLength(0), + ); +}); + +test('should call API when column has advanced data type', async () => { + const props = getAdvancedDataTypeTestProps({ + options: [ + { + type: 'DOUBLE', + column_name: 'advancedDataType', + id: 5, + advanced_data_type: 'type', + }, + ], + }); + + await act(async () => { + render(, { + store, + }); + }); + + const filterValueField = screen.getByPlaceholderText( + 'Filter value (case sensitive)', + ); + await act(async () => { + userEvent.type(filterValueField, 'v'); + }); + + await act(async () => { + userEvent.type(filterValueField, '{enter}'); + }); + + await waitFor(() => + expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_VALID)).toHaveLength(1), + ); + expect(props.validHandler.lastCall.args[0]).toBe(true); +}); + +test('save button should be disabled if error message from API is returned', async () => { + const props = getAdvancedDataTypeTestProps({ + options: [ + { + type: 'DOUBLE', + column_name: 'advancedDataType', + id: 5, + advanced_data_type: 'type', + }, + ], + }); + + await act(async () => { + render(, { + store, + }); + }); + + const filterValueField = screen.getByPlaceholderText( + 'Filter value (case sensitive)', + ); + await act(async () => { + userEvent.type(filterValueField, 'e'); + }); + + await act(async () => { + userEvent.type(filterValueField, '{enter}'); + }); + + await waitFor(() => + expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_INVALID)).toHaveLength( + 1, + ), + ); + expect(props.validHandler.lastCall.args[0]).toBe(false); +}); + +test('advanced data type operator list should update after API response', async () => { + const props = getAdvancedDataTypeTestProps({ + options: [ + { + type: 'DOUBLE', + column_name: 'advancedDataType', + id: 5, + advanced_data_type: 'type', + }, + ], + }); + + await act(async () => { + render(, { + store, + }); + }); + + const filterValueField = screen.getByPlaceholderText( + 'Filter value (case sensitive)', + ); + await act(async () => { + userEvent.type(filterValueField, 'v'); + }); + + await act(async () => { + userEvent.type(filterValueField, '{enter}'); + }); + + await waitFor(() => + expect(fetchMock.calls(ADVANCED_DATA_TYPE_ENDPOINT_VALID)).toHaveLength(1), + ); + expect(props.validHandler.lastCall.args[0]).toBe(true); + + const operatorValueField = screen.getByRole('combobox', { + name: 'Select operator', + }); + + userEvent.click(operatorValueField); + + await act(async () => { + userEvent.type(operatorValueField, '{enter}'); + }); + + expect( + await screen.findByText('Equal to (=)', { + selector: '.ant-select-selection-item', + }), + ).toBeInTheDocument(); +}); + +test('dropdown should remain open when clicked after filter is configured', async () => { + const onChange = sinon.spy(); + const validHandler = sinon.spy(); + const spy = jest.spyOn(redux, 'useSelector'); + spy.mockReturnValue({}); + + const filterWithSubjectAndOperator = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'value', + operatorId: Operators.Equals, + operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.Equals].operation, + comparator: '10', + clause: Clauses.Where, + }); + + const props = { + adhocFilter: filterWithSubjectAndOperator, + onChange, + options, + datasource: { + ...TestDataset, + columns: [{ column_name: 'value', type: 'DOUBLE', id: 3 }], + filter_select: false, + } as Dataset, + partitionColumn: 'test', + validHandler, + }; + + render(); + + const operatorDropdown = screen.getByRole('combobox', { + name: 'Select operator', + }); + + await act(async () => { + userEvent.click(operatorDropdown); + }); + + await waitFor(() => { + expect(operatorDropdown).toHaveAttribute('aria-expanded', 'true'); + }); + + expect(operatorDropdown).toHaveAttribute('aria-expanded', 'true'); +}); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index 150fca4debc..3871a23d0c1 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -16,9 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { FC, ChangeEvent, useEffect, useState } from 'react'; +import { FC, ChangeEvent, useEffect, useState, useRef } from 'react'; -import { Input, Select, Tooltip } from '@superset-ui/core/components'; +import { Input, InputRef, Select, Tooltip } from '@superset-ui/core/components'; import { isFeatureEnabled, FeatureFlag, @@ -263,12 +263,15 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { onComparatorChange, onDatePickerChange, } = useSimpleTabFilterProps(props); + const [comparator, setComparator] = useState(props.adhocFilter.comparator); + const comparatorInputRef = useRef(null); const [suggestions, setSuggestions] = useState< Record<'label' | 'value', any>[] >([]); - const [comparator, setComparator] = useState(props.adhocFilter.comparator); const [loadingComparatorSuggestions, setLoadingComparatorSuggestions] = - useState(false); + useState(false); + const [hasFocusedComparator, setHasFocusedComparator] = + useState(false); const { advancedDataTypesState, @@ -361,7 +364,6 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { notFoundContent: t('Type a value here'), disabled: DISABLE_INPUT_OPERATORS.includes(operatorId), placeholder: createSuggestionsPlaceholder(), - autoFocus: shouldFocusComparator, }; const labelText = @@ -411,16 +413,33 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { }); } }; + if (!datePicker) { refreshComparatorSuggestions(); } - }, [props.adhocFilter.subject]); + // loadingComparatorSuggestions intentionally omitted - set inside effect, would cause infinite loop + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + props.adhocFilter.subject, + props.adhocFilter.clause, + props.datasource, + datePicker, + ]); useEffect(() => { if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) { - fetchSubjectAdvancedDataType(props); + fetchSubjectAdvancedDataType( + props.options, + props.adhocFilter.subject, + props.validHandler, + ); } - }, [props.adhocFilter.subject]); + }, [ + props.adhocFilter.subject, + props.options, + props.validHandler, + fetchSubjectAdvancedDataType, + ]); useEffect(() => { if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) { @@ -430,6 +449,8 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { subjectAdvancedDataType, ); } + // advancedDataTypesState intentionally omitted - set by the callback, would cause infinite API calls + // eslint-disable-next-line react-hooks/exhaustive-deps }, [comparator, subjectAdvancedDataType, fetchAdvancedDataTypeValueCallback]); useEffect(() => { @@ -437,6 +458,22 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { setComparator(props.adhocFilter.comparator); } }, [props.adhocFilter.comparator]); + + useEffect(() => { + if ( + shouldFocusComparator && + !hasFocusedComparator && + comparatorInputRef.current + ) { + comparatorInputRef.current.focus(); + setHasFocusedComparator(true); + } + + if (!shouldFocusComparator) { + setHasFocusedComparator(false); + } + }, [shouldFocusComparator, hasFocusedComparator]); + const theme = useTheme(); // another name for columns, just for following previous naming. @@ -506,11 +543,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { { - if (ref && shouldFocusComparator) { - ref.focus(); - } - }} + ref={comparatorInputRef} onChange={onInputComparatorChange} value={comparator} placeholder={t('Filter value (case sensitive)')} diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/useAdvancedDataTypes.ts b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/useAdvancedDataTypes.ts index 8440b1416dd..91f7bd8263b 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/useAdvancedDataTypes.ts +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/useAdvancedDataTypes.ts @@ -76,20 +76,25 @@ const useAdvancedDataTypes = (validHandler: (isValid: boolean) => void) => { [validHandler], ); - const fetchSubjectAdvancedDataType = (props: Props) => { - const option = props.options.find( - option => - ('column_name' in option && - option.column_name === props.adhocFilter.subject) || - ('optionName' in option && - option.optionName === props.adhocFilter.subject), - ); - if (option && 'advanced_data_type' in option) { - setSubjectAdvancedDataType(option.advanced_data_type); - } else { - props.validHandler(true); - } - }; + const fetchSubjectAdvancedDataType = useCallback( + ( + options: Props['options'], + subject: Props['adhocFilter']['subject'], + validHandler: Props['validHandler'], + ) => { + const option = options.find( + opt => + ('column_name' in opt && opt.column_name === subject) || + ('optionName' in opt && opt.optionName === subject), + ); + if (option && 'advanced_data_type' in option) { + setSubjectAdvancedDataType(option.advanced_data_type); + } else { + validHandler(true); + } + }, + [], + ); return { advancedDataTypesState,