From 16c449748a4b1a0811285ef5c8765cc8b447907b Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Fri, 14 Jun 2024 14:26:58 -0300 Subject: [PATCH] fix: Custom SQL filter control (#29260) --- .../spec/helpers/testing-library.tsx | 24 +++++- .../ScopingModal/ScopingModal.test.tsx | 34 +++------ .../FilterControl/AdhocFilter/index.js | 1 + .../AdhocFilterEditPopover/index.jsx | 5 +- ...hocFilterEditPopoverSqlTabContent.test.jsx | 73 ------------------ ...hocFilterEditPopoverSqlTabContent.test.tsx | 75 +++++++++++++++++++ .../index.jsx | 26 +++---- .../AdhocMetricEditPopover.test.tsx | 7 +- .../rls/RowLevelSecurityModal.test.tsx | 31 ++------ 9 files changed, 132 insertions(+), 144 deletions(-) delete mode 100644 superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.jsx create mode 100644 superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.tsx diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 61ccc7fb0..625531f92 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -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 & { 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); +} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx index 9411d3e7a..392db3ae0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx @@ -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'), ); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js index 627cc50c4..92cb69eeb 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js @@ -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 && diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx index a0e8f33b9..01169ff18 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx @@ -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 { diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.jsx deleted file mode 100644 index 7366e7e0e..000000000 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.jsx +++ /dev/null @@ -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(); - 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); - }); -}); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.tsx new file mode 100644 index 000000000..1d518a5c6 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.tsx @@ -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) => ( +