From d41f9b23a403a5d6882814ac897717b5a33a8f98 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Fri, 18 Mar 2022 11:23:00 -0700 Subject: [PATCH] chore(superset 2.0): remove front-end deprecated code (#19241) --- RESOURCES/FEATURE_FLAGS.md | 1 - superset-frontend/package-lock.json | 16 -- superset-frontend/package.json | 1 - .../TimeFormatterRegistrySingleton.ts | 2 +- .../src/utils/featureFlags.ts | 1 - .../OmniContainer/OmniContainer.test.tsx | 150 ------------------ .../components/OmniContainer/Omnibar.test.tsx | 38 ----- .../src/components/OmniContainer/Omnibar.tsx | 45 ------ .../components/OmniContainer/getDashboards.ts | 54 ------- .../src/components/OmniContainer/index.tsx | 108 ------------- .../src/dashboard/components/Dashboard.jsx | 2 - superset-frontend/src/logger/LogUtils.ts | 2 - .../views/CRUD/dashboard/DashboardList.tsx | 3 - superset/config.py | 2 - 14 files changed, 1 insertion(+), 424 deletions(-) delete mode 100644 superset-frontend/src/components/OmniContainer/OmniContainer.test.tsx delete mode 100644 superset-frontend/src/components/OmniContainer/Omnibar.test.tsx delete mode 100644 superset-frontend/src/components/OmniContainer/Omnibar.tsx delete mode 100644 superset-frontend/src/components/OmniContainer/getDashboards.ts delete mode 100644 superset-frontend/src/components/OmniContainer/index.tsx diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md index 69f8f8a5a6..e2199e86b7 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/RESOURCES/FEATURE_FLAGS.md @@ -41,7 +41,6 @@ These features are **finished** but currently being tested. They are usable, but - DYNAMIC_PLUGINS: [(docs)](https://superset.apache.org/docs/installation/running-on-kubernetes) - DASHBOARD_NATIVE_FILTERS - GLOBAL_ASYNC_QUERIES [(docs)](https://github.com/apache/superset/blob/master/CONTRIBUTING.md#async-chart-queries) -- OMNIBAR - VERSIONED_EXPORT - ENABLE_JAVASCRIPT_CONTROLS diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index c6831265e0..24c235692a 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -89,7 +89,6 @@ "moment-timezone": "^0.5.33", "mousetrap": "^1.6.1", "mustache": "^2.2.1", - "omnibar": "^2.1.1", "polished": "^3.7.2", "prop-types": "^15.7.2", "query-string": "^6.13.7", @@ -45525,15 +45524,6 @@ "resolved": "https://registry.npmjs.org/omit.js/-/omit.js-2.0.2.tgz", "integrity": "sha512-hJmu9D+bNB40YpL9jYebQl4lsTW6yEHRTroJzNLqQJYHm7c+NQnJGfZmIWh8S3q3KoaxV1aLhV6B3+0N0/kyJg==" }, - "node_modules/omnibar": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/omnibar/-/omnibar-2.1.1.tgz", - "integrity": "sha512-8txe0of2sb6amV+0vB/VbF7kzwQF8vo9lwmzZTt7TIuugWI661STacLORfl4O6r/A9c4fu69o8Rb+6HIdqArEQ==", - "peerDependencies": { - "react": "^15.5.4", - "react-dom": "^15.5.4" - } - }, "node_modules/on-finished": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.3.0.tgz", @@ -95703,12 +95693,6 @@ "resolved": "https://registry.npmjs.org/omit.js/-/omit.js-2.0.2.tgz", "integrity": "sha512-hJmu9D+bNB40YpL9jYebQl4lsTW6yEHRTroJzNLqQJYHm7c+NQnJGfZmIWh8S3q3KoaxV1aLhV6B3+0N0/kyJg==" }, - "omnibar": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/omnibar/-/omnibar-2.1.1.tgz", - "integrity": "sha512-8txe0of2sb6amV+0vB/VbF7kzwQF8vo9lwmzZTt7TIuugWI661STacLORfl4O6r/A9c4fu69o8Rb+6HIdqArEQ==", - "requires": {} - }, "on-finished": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.3.0.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 4dabb388c1..d8161d0d7d 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -149,7 +149,6 @@ "moment-timezone": "^0.5.33", "mousetrap": "^1.6.1", "mustache": "^2.2.1", - "omnibar": "^2.1.1", "polished": "^3.7.2", "prop-types": "^15.7.2", "query-string": "^6.13.7", diff --git a/superset-frontend/packages/superset-ui-core/src/time-format/TimeFormatterRegistrySingleton.ts b/superset-frontend/packages/superset-ui-core/src/time-format/TimeFormatterRegistrySingleton.ts index 2eae7a41b5..c9aaa2e9a1 100644 --- a/superset-frontend/packages/superset-ui-core/src/time-format/TimeFormatterRegistrySingleton.ts +++ b/superset-frontend/packages/superset-ui-core/src/time-format/TimeFormatterRegistrySingleton.ts @@ -75,7 +75,7 @@ export function getTimeFormatter( /** * Syntactic sugar for backward compatibility - * TODO: Deprecate this in the next breaking change. + * TODO: will be deprecated in a future version * @param granularity */ export function getTimeFormatterForGranularity(granularity?: TimeGranularity) { diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 8ed617cc3e..341561f33a 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -21,7 +21,6 @@ export enum FeatureFlag { ALLOW_DASHBOARD_DOMAIN_SHARDING = 'ALLOW_DASHBOARD_DOMAIN_SHARDING', ALERT_REPORTS = 'ALERT_REPORTS', - OMNIBAR = 'OMNIBAR', CLIENT_CACHE = 'CLIENT_CACHE', DYNAMIC_PLUGINS = 'DYNAMIC_PLUGINS', SCHEDULED_QUERIES = 'SCHEDULED_QUERIES', diff --git a/superset-frontend/src/components/OmniContainer/OmniContainer.test.tsx b/superset-frontend/src/components/OmniContainer/OmniContainer.test.tsx deleted file mode 100644 index dd926b632d..0000000000 --- a/superset-frontend/src/components/OmniContainer/OmniContainer.test.tsx +++ /dev/null @@ -1,150 +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. - */ - -import React from 'react'; -import { render, screen, fireEvent } from 'spec/helpers/testing-library'; -import { isFeatureEnabled } from 'src/featureFlags'; -import OmniContainer from './index'; - -jest.mock('src/featureFlags', () => ({ - isFeatureEnabled: jest.fn(), - FeatureFlag: { OMNIBAR: 'OMNIBAR' }, - initFeatureFlags: jest.fn(), -})); - -test('Do not open Omnibar with the featureflag disabled', () => { - (isFeatureEnabled as jest.Mock).mockImplementation( - (ff: string) => !(ff === 'OMNIBAR'), - ); - render( -
- -
, - ); - - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); - fireEvent.keyDown(screen.getByTestId('test'), { - ctrlKey: true, - code: 'KeyK', - }); - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); -}); - -test('Open Omnibar with ctrl + k with featureflag enabled', () => { - (isFeatureEnabled as jest.Mock).mockImplementation( - (ff: string) => ff === 'OMNIBAR', - ); - render( -
- -
, - ); - - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); - - // show Omnibar - fireEvent.keyDown(screen.getByTestId('test'), { - ctrlKey: true, - code: 'KeyK', - }); - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).toBeInTheDocument(); - - // hide Omnibar - fireEvent.keyDown(screen.getByTestId('test'), { - ctrlKey: true, - code: 'KeyK', - }); - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); -}); - -test('Open Omnibar with Command + k with featureflag enabled', () => { - (isFeatureEnabled as jest.Mock).mockImplementation( - (ff: string) => ff === 'OMNIBAR', - ); - render( -
- -
, - ); - - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); - - // show Omnibar - fireEvent.keyDown(screen.getByTestId('test'), { - metaKey: true, - code: 'KeyK', - }); - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).toBeInTheDocument(); - - // hide Omnibar - fireEvent.keyDown(screen.getByTestId('test'), { - metaKey: true, - code: 'KeyK', - }); - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); -}); - -test('Open Omnibar with Cmd/Ctrl-K and close with ESC', () => { - (isFeatureEnabled as jest.Mock).mockImplementation( - (ff: string) => ff === 'OMNIBAR', - ); - render( -
- -
, - ); - - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); - - // show Omnibar - fireEvent.keyDown(screen.getByTestId('test'), { - ctrlKey: true, - code: 'KeyK', - }); - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).toBeInTheDocument(); - - // Close Omnibar - fireEvent.keyDown(screen.getByTestId('test'), { - key: 'Escape', - code: 'Escape', - }); - expect( - screen.queryByPlaceholderText('Search all dashboards'), - ).not.toBeInTheDocument(); -}); diff --git a/superset-frontend/src/components/OmniContainer/Omnibar.test.tsx b/superset-frontend/src/components/OmniContainer/Omnibar.test.tsx deleted file mode 100644 index 39b367326a..0000000000 --- a/superset-frontend/src/components/OmniContainer/Omnibar.test.tsx +++ /dev/null @@ -1,38 +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. - */ - -import React from 'react'; -import { render, screen } from 'spec/helpers/testing-library'; -import { Omnibar } from './Omnibar'; - -test('Must put id on input', () => { - render( - , - ); - - expect(screen.getByPlaceholderText('Test Omnibar')).toBeInTheDocument(); - expect(screen.getByPlaceholderText('Test Omnibar')).toHaveAttribute( - 'id', - 'test-id-attribute', - ); -}); diff --git a/superset-frontend/src/components/OmniContainer/Omnibar.tsx b/superset-frontend/src/components/OmniContainer/Omnibar.tsx deleted file mode 100644 index aeeffd1ef6..0000000000 --- a/superset-frontend/src/components/OmniContainer/Omnibar.tsx +++ /dev/null @@ -1,45 +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. - */ - -import React from 'react'; -import OmnibarDeprecated from 'omnibar'; - -interface Props { - id: string; - placeholder: string; - extensions: ((query: string) => Promise)[]; -} - -/** - * @deprecated Component "omnibar" does not support prop className or id (the original implementation used className). However, the original javascript code was sending these prop and was working correctly. lol - * As this behavior is unpredictable, and does not works whitch types, I have isolated this component so that in the future a better solution can be found and implemented. - * We need to find a substitute for this component or some way of working around this problem - */ -export function Omnibar({ extensions, placeholder, id }: Props) { - return ( - - ); -} diff --git a/superset-frontend/src/components/OmniContainer/getDashboards.ts b/superset-frontend/src/components/OmniContainer/getDashboards.ts deleted file mode 100644 index faa8336b0b..0000000000 --- a/superset-frontend/src/components/OmniContainer/getDashboards.ts +++ /dev/null @@ -1,54 +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. - */ - -import { t, SupersetClient } from '@superset-ui/core'; - -interface DashboardItem { - changed_by_name: string; - changed_on: string; - creator: string; - dashboard_link: string; - dashboard_title: string; - id: number; - modified: string; - url: string; -} - -interface Dashboards extends DashboardItem { - title: string; -} - -export const getDashboards = async ( - query: string, -): Promise<(Dashboards | { title: string })[]> => { - // todo: Build a dedicated endpoint for dashboard searching - // i.e. superset/v1/api/dashboards?q=${query} - let response; - try { - response = await SupersetClient.get({ - endpoint: `/dashboardasync/api/read?_oc_DashboardModelViewAsync=changed_on&_od_DashboardModelViewAsync=desc&_flt_2_dashboard_title=${query}`, - }); - } catch (error) { - return [{ title: t('An error occurred while fetching dashboards') }]; - } - return response?.json.result.map((item: DashboardItem) => ({ - title: item.dashboard_title, - ...item, - })); -}; diff --git a/superset-frontend/src/components/OmniContainer/index.tsx b/superset-frontend/src/components/OmniContainer/index.tsx deleted file mode 100644 index c5ca25c380..0000000000 --- a/superset-frontend/src/components/OmniContainer/index.tsx +++ /dev/null @@ -1,108 +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. - */ - -import React, { useRef, useState } from 'react'; -import { styled, t } from '@superset-ui/core'; -import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; -import Modal from 'src/components/Modal'; -import { useComponentDidMount } from 'src/hooks/useComponentDidMount'; -import { logEvent } from 'src/logger/actions'; -import { Omnibar } from './Omnibar'; -import { LOG_ACTIONS_OMNIBAR_TRIGGERED } from '../../logger/LogUtils'; -import { getDashboards } from './getDashboards'; - -const OmniModal = styled(Modal)` - margin-top: 20%; - - .ant-modal-body { - padding: 0; - overflow: visible; - } -`; - -export default function OmniContainer() { - const showOmni = useRef(); - const modalRef = useRef(null); - const [showModal, setShowModal] = useState(false); - const handleLogEvent = (show: boolean) => - logEvent(LOG_ACTIONS_OMNIBAR_TRIGGERED, { - show_omni: show, - }); - const handleClose = () => { - showOmni.current = false; - setShowModal(false); - handleLogEvent(false); - }; - - useComponentDidMount(() => { - showOmni.current = false; - - function handleKeydown(event: KeyboardEvent) { - if (!isFeatureEnabled(FeatureFlag.OMNIBAR)) return; - const controlOrCommand = event.ctrlKey || event.metaKey; - const isOk = ['KeyK'].includes(event.code); - const isEsc = event.key === 'Escape'; - - if (isEsc && showOmni.current) { - handleClose(); - return; - } - if (controlOrCommand && isOk) { - showOmni.current = !showOmni.current; - setShowModal(showOmni.current); - handleLogEvent(!!showOmni.current); - } - } - - function handleClickOutside(event: MouseEvent) { - if ( - modalRef.current && - !modalRef.current.contains(event.target as Node) - ) { - handleClose(); - } - } - - document.addEventListener('mousedown', handleClickOutside); - document.addEventListener('keydown', handleKeydown); - return () => { - document.removeEventListener('keydown', handleKeydown); - document.removeEventListener('mousedown', handleClickOutside); - }; - }); - - return ( - {}} - destroyOnClose - > -
- -
- - ); -} diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index 22c20883fe..f04eb696aa 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.jsx @@ -36,7 +36,6 @@ import { LOG_ACTIONS_MOUNT_DASHBOARD, Logger, } from '../../logger/LogUtils'; -import OmniContainer from '../../components/OmniContainer'; import { areObjectsEqual } from '../../reduxUtils'; import '../stylesheets/index.less'; @@ -292,7 +291,6 @@ class Dashboard extends React.PureComponent { } return ( <> - ); diff --git a/superset-frontend/src/logger/LogUtils.ts b/superset-frontend/src/logger/LogUtils.ts index 9cdb5e4634..b3e834bc93 100644 --- a/superset-frontend/src/logger/LogUtils.ts +++ b/superset-frontend/src/logger/LogUtils.ts @@ -35,7 +35,6 @@ export const LOG_ACTIONS_EXPLORE_DASHBOARD_CHART = 'explore_dashboard_chart'; export const LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART = 'export_csv_dashboard_chart'; export const LOG_ACTIONS_CHANGE_DASHBOARD_FILTER = 'change_dashboard_filter'; -export const LOG_ACTIONS_OMNIBAR_TRIGGERED = 'omnibar_triggered'; // Log event types -------------------------------------------------------------- export const LOG_EVENT_TYPE_TIMING = new Set([ @@ -54,7 +53,6 @@ export const LOG_EVENT_TYPE_USER = new Set([ LOG_ACTIONS_TOGGLE_EDIT_DASHBOARD, LOG_ACTIONS_FORCE_REFRESH_DASHBOARD, LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD, - LOG_ACTIONS_OMNIBAR_TRIGGERED, LOG_ACTIONS_MOUNT_EXPLORER, ]); diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index aa1b9fc7b7..b3da8ee8e3 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -46,7 +46,6 @@ import FaveStar from 'src/components/FaveStar'; import PropertiesModal from 'src/dashboard/components/PropertiesModal'; import { Tooltip } from 'src/components/Tooltip'; import ImportModelsModal from 'src/components/ImportModal/index'; -import OmniContainer from 'src/components/OmniContainer'; import Dashboard from 'src/dashboard/containers/Dashboard'; import CertifiedBadge from 'src/components/CertifiedBadge'; @@ -689,8 +688,6 @@ function DashboardList(props: DashboardListProps) { setPasswordFields={setPasswordFields} /> - - {preparingExport && } ); diff --git a/superset/config.py b/superset/config.py index 6a84a1cf40..576428f06b 100644 --- a/superset/config.py +++ b/superset/config.py @@ -420,8 +420,6 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = { "EMBEDDED_SUPERSET": False, # Enables Alerts and reports new implementation "ALERT_REPORTS": False, - # Enable experimental feature to search for other dashboards - "OMNIBAR": False, "DASHBOARD_RBAC": False, "ENABLE_EXPLORE_DRAG_AND_DROP": True, "ENABLE_FILTER_BOX_MIGRATION": False,