fix: Custom SQL filter control (#29260)

This commit is contained in:
Michael S. Molina 2024-06-14 14:26:58 -03:00 committed by GitHub
parent 2418efe85c
commit 16c449748a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 132 additions and 144 deletions

View File

@ -18,7 +18,13 @@
*/
import '@testing-library/jest-dom/extend-expect';
import { ReactNode, ReactElement } from 'react';
import { render, RenderOptions } from '@testing-library/react';
import {
render,
RenderOptions,
screen,
waitFor,
within,
} from '@testing-library/react';
import { ThemeProvider, supersetTheme } from '@superset-ui/core';
import { BrowserRouter } from 'react-router-dom';
import { Provider } from 'react-redux';
@ -28,6 +34,7 @@ import reducerIndex from 'spec/helpers/reducerIndex';
import { QueryParamProvider } from 'use-query-params';
import { configureStore, Store } from '@reduxjs/toolkit';
import { api } from 'src/hooks/apiResources/queryApi';
import userEvent from '@testing-library/user-event';
type Options = Omit<RenderOptions, 'queries'> & {
useRedux?: boolean;
@ -102,3 +109,18 @@ export function sleep(time: number) {
export * from '@testing-library/react';
export { customRender as render };
export async function selectOption(option: string, selectName?: string) {
const select = screen.getByRole(
'combobox',
selectName ? { name: selectName } : {},
);
await userEvent.click(select);
const item = await waitFor(() =>
within(
// eslint-disable-next-line testing-library/no-node-access
document.querySelector('.rc-virtual-list')!,
).getByText(option),
);
await userEvent.click(item);
}

View File

@ -18,7 +18,13 @@
*/
import fetchMock from 'fetch-mock';
import userEvent from '@testing-library/user-event';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import {
render,
screen,
selectOption,
waitFor,
within,
} from 'spec/helpers/testing-library';
import {
CHART_TYPE,
DASHBOARD_ROOT_TYPE,
@ -199,21 +205,7 @@ it('add new custom scoping', async () => {
expect(screen.getByText('[new custom scoping]')).toBeInTheDocument();
expect(screen.getByText('[new custom scoping]')).toHaveClass('active');
await waitFor(() =>
userEvent.click(screen.getByRole('combobox', { name: 'Select chart' })),
);
await waitFor(() => {
userEvent.click(
within(document.querySelector('.rc-virtual-list')!).getByText('chart 1'),
);
});
expect(
within(document.querySelector('.ant-select-selection-item')!).getByText(
'chart 1',
),
).toBeInTheDocument();
await selectOption('chart 1', 'Select chart');
expect(
document.querySelectorAll(
'[data-test="scoping-tree-panel"] .ant-tree-checkbox-checked',
@ -250,14 +242,8 @@ it('edit scope and save', async () => {
// create custom scoping for chart 1 with unselected chart 2 (from global) and chart 4
userEvent.click(screen.getByText('Add custom scoping'));
await waitFor(() =>
userEvent.click(screen.getByRole('combobox', { name: 'Select chart' })),
);
await waitFor(() => {
userEvent.click(
within(document.querySelector('.rc-virtual-list')!).getByText('chart 1'),
);
});
await selectOption('chart 1', 'Select chart');
userEvent.click(
within(document.querySelector('.ant-tree')!).getByText('chart 4'),
);

View File

@ -92,6 +92,7 @@ export default class AdhocFilter {
equals(adhocFilter) {
return (
adhocFilter.clause === this.clause &&
adhocFilter.expressionType === this.expressionType &&
adhocFilter.sqlExpression === this.sqlExpression &&
adhocFilter.operator === this.operator &&

View File

@ -73,11 +73,12 @@ const FilterPopoverContentContainer = styled.div`
.filter-edit-clause-info {
font-size: ${({ theme }) => theme.typography.sizes.xs}px;
padding-left: ${({ theme }) => theme.gridUnit}px;
}
.filter-edit-clause-section {
display: inline-flex;
display: flex;
flex-direction: row;
gap: ${({ theme }) => theme.gridUnit * 5}px;
}
.adhoc-filter-simple-column-dropdown {

View File

@ -1,73 +0,0 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
/* eslint-disable no-unused-expressions */
import sinon from 'sinon';
import { shallow } from 'enzyme';
import AdhocFilter from 'src/explore/components/controls/FilterControl/AdhocFilter';
import AdhocFilterEditPopoverSqlTabContent from '.';
import { Clauses, ExpressionTypes } from '../types';
const sqlAdhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Sql,
sqlExpression: 'value > 10',
clause: Clauses.Where,
});
function setup(overrides) {
const onChange = sinon.spy();
const props = {
adhocFilter: sqlAdhocFilter,
onChange,
options: [],
height: 100,
...overrides,
};
const wrapper = shallow(<AdhocFilterEditPopoverSqlTabContent {...props} />);
return { wrapper, onChange };
}
describe('AdhocFilterEditPopoverSqlTabContent', () => {
it('renders the sql tab form', () => {
const { wrapper } = setup();
expect(wrapper).toExist();
});
it('passes the new clause to onChange after onSqlExpressionClauseChange', () => {
const { wrapper, onChange } = setup();
wrapper.instance().onSqlExpressionClauseChange(Clauses.Having);
expect(onChange.calledOnce).toBe(true);
expect(
onChange.lastCall.args[0].equals(
sqlAdhocFilter.duplicateWith({ clause: Clauses.Having }),
),
).toBe(true);
});
it('passes the new query to onChange after onSqlExpressionChange', () => {
const { wrapper, onChange } = setup();
wrapper.instance().onSqlExpressionChange('value < 5');
expect(onChange.calledOnce).toBe(true);
expect(
onChange.lastCall.args[0].equals(
sqlAdhocFilter.duplicateWith({ sqlExpression: 'value < 5' }),
),
).toBe(true);
});
});

View File

@ -0,0 +1,75 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { render, screen, selectOption } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { IAceEditorProps } from 'react-ace';
import AdhocFilter from '../AdhocFilter';
import { Clauses, ExpressionTypes } from '../types';
import AdhocFilterEditPopoverSqlTabContent from '.';
jest.mock('src/components/AsyncAceEditor', () => ({
SQLEditor: ({ value, onChange }: IAceEditorProps) => (
<textarea
defaultValue={value}
onChange={value => onChange?.(value.target.value)}
/>
),
}));
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Sql,
sqlExpression: 'value > 10',
clause: Clauses.Where,
});
test('calls onChange when the SQL clause changes', async () => {
const onChange = jest.fn();
render(
<AdhocFilterEditPopoverSqlTabContent
adhocFilter={adhocFilter}
onChange={onChange}
options={[]}
height={100}
/>,
);
await selectOption(Clauses.Having);
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({ clause: Clauses.Having }),
);
});
test('calls onChange when the SQL expression changes', () => {
const onChange = jest.fn();
const input = 'value < 20';
render(
<AdhocFilterEditPopoverSqlTabContent
adhocFilter={adhocFilter}
onChange={onChange}
options={[]}
height={100}
/>,
);
const sqlEditor = screen.getByRole('textbox');
expect(sqlEditor).toBeInTheDocument();
userEvent.clear(sqlEditor);
userEvent.paste(sqlEditor, input);
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({ sqlExpression: input }),
);
});

View File

@ -39,7 +39,6 @@ const propTypes = {
]),
).isRequired,
height: PropTypes.number.isRequired,
activeKey: PropTypes.string.isRequired,
};
const StyledSelect = styled(Select)`
@ -56,10 +55,6 @@ export default class AdhocFilterEditPopoverSqlTabContent extends Component {
this.onSqlExpressionClauseChange =
this.onSqlExpressionClauseChange.bind(this);
this.handleAceEditorRef = this.handleAceEditorRef.bind(this);
this.selectProps = {
ariaLabel: t('Select column'),
};
}
componentDidUpdate() {
@ -95,11 +90,6 @@ export default class AdhocFilterEditPopoverSqlTabContent extends Component {
render() {
const { adhocFilter, height, options } = this.props;
const clauseSelectProps = {
placeholder: t('choose WHERE or HAVING...'),
value: adhocFilter.clause,
onChange: this.onSqlExpressionClauseChange,
};
const keywords = sqlKeywords.concat(
options
.map(option => {
@ -115,7 +105,7 @@ export default class AdhocFilterEditPopoverSqlTabContent extends Component {
})
.filter(Boolean),
);
const selectOptions = Object.keys(Clauses).map(clause => ({
const selectOptions = Object.values(Clauses).map(clause => ({
label: clause,
value: clause,
}));
@ -123,11 +113,15 @@ export default class AdhocFilterEditPopoverSqlTabContent extends Component {
return (
<span>
<div className="filter-edit-clause-section">
<StyledSelect
options={selectOptions}
{...this.selectProps}
{...clauseSelectProps}
/>
<div>
<StyledSelect
options={selectOptions}
ariaLabel={t('Select column')}
placeholder={t('choose WHERE or HAVING...')}
value={adhocFilter.clause}
onChange={this.onSqlExpressionClauseChange}
/>
</div>
<span className="filter-edit-clause-info">
<strong>WHERE</strong> {t('Filters by columns')}
<br />

View File

@ -17,7 +17,7 @@
* under the License.
*/
import userEvent from '@testing-library/user-event';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import { render, screen, selectOption } from 'spec/helpers/testing-library';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
import AdhocMetricEditPopover from '.';
@ -132,10 +132,7 @@ test('Clicking on "Save" should call onChange and onClose', async () => {
name: 'Select saved metrics',
}),
);
const sumOption = await waitFor(() =>
within(document.querySelector('.rc-virtual-list')!).getByText('sum'),
);
userEvent.click(sumOption);
await selectOption('sum');
userEvent.click(screen.getByRole('button', { name: 'Save' }));
expect(props.onChange).toBeCalledTimes(1);
expect(props.onClose).toBeCalledTimes(1);

View File

@ -17,7 +17,12 @@
* under the License.
*/
import fetchMock from 'fetch-mock';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import {
render,
screen,
selectOption,
waitFor,
} from 'spec/helpers/testing-library';
import { act } from 'react-dom/test-utils';
import userEvent from '@testing-library/user-event';
import RowLevelSecurityModal, {
@ -230,17 +235,7 @@ describe('Rule modal', () => {
expect(addButton).toBeDisabled();
const getSelect = () => screen.getByRole('combobox', { name: 'Tables' });
const getElementByClassName = (className: string) =>
document.querySelector(className)! as HTMLElement;
const findSelectOption = (text: string) =>
waitFor(() =>
within(getElementByClassName('.rc-virtual-list')).getByText(text),
);
const open = () => waitFor(() => userEvent.click(getSelect()));
await open();
userEvent.click(await findSelectOption('birth_names'));
await selectOption('birth_names', 'Tables');
expect(addButton).toBeDisabled();
const clause = await screen.findByTestId('clause-test');
@ -257,17 +252,7 @@ describe('Rule modal', () => {
const nameTextBox = screen.getByTestId('rule-name-test');
userEvent.type(nameTextBox, 'name');
const getSelect = () => screen.getByRole('combobox', { name: 'Tables' });
const getElementByClassName = (className: string) =>
document.querySelector(className)! as HTMLElement;
const findSelectOption = (text: string) =>
waitFor(() =>
within(getElementByClassName('.rc-virtual-list')).getByText(text),
);
const open = () => waitFor(() => userEvent.click(getSelect()));
await open();
userEvent.click(await findSelectOption('birth_names'));
await selectOption('birth_names', 'Tables');
const clause = await screen.findByTestId('clause-test');
userEvent.type(clause, 'gender="girl"');