From 8045c3eb2f861c600d2481609a1c12ad63f01991 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 7 Mar 2022 14:35:43 -0800 Subject: [PATCH] =?UTF-8?q?Revert=20"feat:=20DBC-UI=20Globally=20available?= =?UTF-8?q?=20across=20the=20app=20=F0=9F=8C=8E=20(#18722)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 209e3f45548ae8d0b3ac63d2c393883b733d2b22. --- .../src/views/components/Menu.test.tsx | 99 ++------- .../src/views/components/Menu.tsx | 2 +- .../src/views/components/MenuRight.tsx | 189 ++++++++---------- .../src/views/components/types.ts | 38 ---- superset/views/base.py | 6 - 5 files changed, 102 insertions(+), 232 deletions(-) delete mode 100644 superset-frontend/src/views/components/types.ts diff --git a/superset-frontend/src/views/components/Menu.test.tsx b/superset-frontend/src/views/components/Menu.test.tsx index d13275fbc0b57..b990ddc1804a9 100644 --- a/superset-frontend/src/views/components/Menu.test.tsx +++ b/superset-frontend/src/views/components/Menu.test.tsx @@ -21,64 +21,7 @@ import * as reactRedux from 'react-redux'; import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { Menu } from './Menu'; - -const dropdownItems = [ - { - label: 'Data', - icon: 'fa-database', - childs: [ - { - label: 'Connect Database', - name: 'dbconnect', - perm: true, - }, - { - label: 'Connect Google Sheet', - name: 'gsheets', - perm: true, - }, - { - label: 'Upload a CSV', - name: 'Upload a CSV', - url: '/csvtodatabaseview/form', - perm: true, - }, - { - label: 'Upload a Columnar File', - name: 'Upload a Columnar file', - url: '/columnartodatabaseview/form', - perm: true, - }, - { - label: 'Upload Excel', - name: 'Upload Excel', - url: '/exceltodatabaseview/form', - perm: true, - }, - ], - }, - { - label: 'SQL query', - url: '/superset/sqllab?new=true', - icon: 'fa-fw fa-search', - perm: 'can_sqllab', - view: 'Superset', - }, - { - label: 'Chart', - url: '/chart/add', - icon: 'fa-fw fa-bar-chart', - perm: 'can_write', - view: 'Chart', - }, - { - label: 'Dashboard', - url: '/dashboard/new', - icon: 'fa-fw fa-dashboard', - perm: 'can_write', - view: 'Dashboard', - }, -]; +import { dropdownItems } from './MenuRight'; const user = { createdOn: '2021-04-27T18:12:38.952304', @@ -242,13 +185,13 @@ beforeEach(() => { test('should render', () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - const { container } = render(, { useRedux: true }); + const { container } = render(); expect(container).toBeInTheDocument(); }); test('should render the navigation', () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); expect(screen.getByRole('navigation')).toBeInTheDocument(); }); @@ -259,7 +202,7 @@ test('should render the brand', () => { brand: { alt, icon }, }, } = mockedProps; - render(, { useRedux: true }); + render(); const image = screen.getByAltText(alt); expect(image).toHaveAttribute('src', icon); }); @@ -269,7 +212,7 @@ test('should render all the top navbar menu items', () => { const { data: { menu }, } = mockedProps; - render(, { useRedux: true }); + render(); menu.forEach(item => { expect(screen.getByText(item.label)).toBeInTheDocument(); }); @@ -280,7 +223,7 @@ test('should render the top navbar child menu items', async () => { const { data: { menu }, } = mockedProps; - render(, { useRedux: true }); + render(); const sources = screen.getByText('Sources'); userEvent.hover(sources); const datasets = await screen.findByText('Datasets'); @@ -294,7 +237,7 @@ test('should render the top navbar child menu items', async () => { test('should render the dropdown items', async () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); const dropdown = screen.getByTestId('new-dropdown-icon'); userEvent.hover(dropdown); // todo (philip): test data submenu @@ -320,14 +263,14 @@ test('should render the dropdown items', async () => { test('should render the Settings', async () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); const settings = await screen.findByText('Settings'); expect(settings).toBeInTheDocument(); }); test('should render the Settings menu item', async () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); userEvent.hover(screen.getByText('Settings')); const label = await screen.findByText('Security'); expect(label).toBeInTheDocument(); @@ -338,7 +281,7 @@ test('should render the Settings dropdown child menu items', async () => { const { data: { settings }, } = mockedProps; - render(, { useRedux: true }); + render(); userEvent.hover(screen.getByText('Settings')); const listUsers = await screen.findByText('List Users'); expect(listUsers).toHaveAttribute('href', settings[0].childs[0].url); @@ -346,13 +289,13 @@ test('should render the Settings dropdown child menu items', async () => { test('should render the plus menu (+) when user is not anonymous', () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); expect(screen.getByTestId('new-dropdown')).toBeInTheDocument(); }); test('should NOT render the plus menu (+) when user is anonymous', () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument(); }); @@ -364,7 +307,7 @@ test('should render the user actions when user is not anonymous', async () => { }, } = mockedProps; - render(, { useRedux: true }); + render(); userEvent.hover(screen.getByText('Settings')); const user = await screen.findByText('User'); expect(user).toBeInTheDocument(); @@ -378,7 +321,7 @@ test('should render the user actions when user is not anonymous', async () => { test('should NOT render the user actions when user is anonymous', () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); expect(screen.queryByText('User')).not.toBeInTheDocument(); }); @@ -390,7 +333,7 @@ test('should render the Profile link when available', async () => { }, } = mockedProps; - render(, { useRedux: true }); + render(); userEvent.hover(screen.getByText('Settings')); const profile = await screen.findByText('Profile'); @@ -405,7 +348,7 @@ test('should render the About section and version_string, sha or build_number wh }, } = mockedProps; - render(, { useRedux: true }); + render(); userEvent.hover(screen.getByText('Settings')); const about = await screen.findByText('About'); const version = await screen.findByText(`Version: ${version_string}`); @@ -424,7 +367,7 @@ test('should render the Documentation link when available', async () => { navbar_right: { documentation_url }, }, } = mockedProps; - render(, { useRedux: true }); + render(); userEvent.hover(screen.getByText('Settings')); const doc = await screen.findByTitle('Documentation'); expect(doc).toHaveAttribute('href', documentation_url); @@ -438,7 +381,7 @@ test('should render the Bug Report link when available', async () => { }, } = mockedProps; - render(, { useRedux: true }); + render(); const bugReport = await screen.findByTitle('Report a bug'); expect(bugReport).toHaveAttribute('href', bug_report_url); }); @@ -451,19 +394,19 @@ test('should render the Login link when user is anonymous', () => { }, } = mockedProps; - render(, { useRedux: true }); + render(); const login = screen.getByText('Login'); expect(login).toHaveAttribute('href', user_login_url); }); test('should render the Language Picker', () => { useSelectorMock.mockReturnValue({ roles: user.roles }); - render(, { useRedux: true }); + render(); expect(screen.getByLabelText('Languages')).toBeInTheDocument(); }); test('should hide create button without proper roles', () => { useSelectorMock.mockReturnValue({ roles: [] }); - render(, { useRedux: true }); + render(); expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument(); }); diff --git a/superset-frontend/src/views/components/Menu.tsx b/superset-frontend/src/views/components/Menu.tsx index 47414ba09993f..0b562e6ba81bf 100644 --- a/superset-frontend/src/views/components/Menu.tsx +++ b/superset-frontend/src/views/components/Menu.tsx @@ -74,7 +74,7 @@ interface MenuObjectChildProps { index?: number; url?: string; isFrontendRoute?: boolean; - perm?: string | boolean; + perm?: string; view?: string; } diff --git a/superset-frontend/src/views/components/MenuRight.tsx b/superset-frontend/src/views/components/MenuRight.tsx index 7f8adce6bf931..42bad7a7e5029 100644 --- a/superset-frontend/src/views/components/MenuRight.tsx +++ b/superset-frontend/src/views/components/MenuRight.tsx @@ -16,22 +16,67 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState } from 'react'; +import React from 'react'; import { MainNav as Menu } from 'src/components/Menu'; import { t, styled, css, SupersetTheme } from '@superset-ui/core'; import { Link } from 'react-router-dom'; import Icons from 'src/components/Icons'; import findPermission from 'src/dashboard/util/findPermission'; import { useSelector } from 'react-redux'; -import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; -import LanguagePicker from './LanguagePicker'; -import DatabaseModal from '../CRUD/data/database/DatabaseModal'; import { - ExtentionConfigs, - GlobalMenuDataOptions, - RightMenuProps, -} from './types'; -import { MenuObjectProps } from './Menu'; + UserWithPermissionsAndRoles, + CommonBootstrapData, +} from 'src/types/bootstrapTypes'; +import LanguagePicker from './LanguagePicker'; +import { NavBarProps, MenuObjectProps } from './Menu'; + +export const dropdownItems: MenuObjectProps[] = [ + { + label: t('Data'), + icon: 'fa-database', + childs: [ + { + icon: 'fa-upload', + label: t('Upload a CSV'), + name: 'Upload a CSV', + url: '/csvtodatabaseview/form', + }, + { + icon: 'fa-upload', + label: t('Upload a Columnar File'), + name: 'Upload a Columnar file', + url: '/columnartodatabaseview/form', + }, + { + icon: 'fa-upload', + label: t('Upload Excel'), + name: 'Upload Excel', + url: '/exceltodatabaseview/form', + }, + ], + }, + { + label: t('SQL query'), + url: '/superset/sqllab?new=true', + icon: 'fa-fw fa-search', + perm: 'can_sqllab', + view: 'Superset', + }, + { + label: t('Chart'), + url: '/chart/add', + icon: 'fa-fw fa-bar-chart', + perm: 'can_write', + view: 'Chart', + }, + { + label: t('Dashboard'), + url: '/dashboard/new', + icon: 'fa-fw fa-dashboard', + perm: 'can_write', + view: 'Dashboard', + }, +]; const versionInfoStyles = (theme: SupersetTheme) => css` padding: ${theme.gridUnit * 1.5}px ${theme.gridUnit * 4}px @@ -62,6 +107,13 @@ const StyledAnchor = styled.a` const { SubMenu } = Menu; +interface RightMenuProps { + align: 'flex-start' | 'flex-end'; + settings: MenuObjectProps[]; + navbarRight: NavBarProps; + isFrontendRoute: (path?: string) => boolean; +} + const RightMenu = ({ align, settings, @@ -71,106 +123,30 @@ const RightMenu = ({ const { roles } = useSelector( state => state.user, ); - const { - CSV_EXTENSIONS, - COLUMNAR_EXTENSIONS, - EXCEL_EXTENSIONS, - HAS_GSHEETS_INSTALLED, - } = useSelector(state => state.common.conf); - - const [showModal, setShowModal] = useState(false); - const [engine, setEngine] = useState(''); + // @ts-ignore + const { CSV_EXTENSIONS, COLUMNAR_EXTENSIONS, EXCEL_EXTENSIONS } = useSelector< + any, + CommonBootstrapData + >(state => state.common.conf); + // if user has any of these roles the dropdown will appear + const configMap = { + 'Upload a CSV': CSV_EXTENSIONS, + 'Upload a Columnar file': COLUMNAR_EXTENSIONS, + 'Upload Excel': EXCEL_EXTENSIONS, + }; const canSql = findPermission('can_sqllab', 'Superset', roles); const canDashboard = findPermission('can_write', 'Dashboard', roles); const canChart = findPermission('can_write', 'Chart', roles); const showActionDropdown = canSql || canChart || canDashboard; - const dropdownItems: MenuObjectProps[] = [ - { - label: t('Data'), - icon: 'fa-database', - childs: [ - { - label: t('Connect database'), - name: GlobalMenuDataOptions.DB_CONNECTION, - perm: true, - }, - { - label: t('Connect Google Sheet'), - name: GlobalMenuDataOptions.GOOGLE_SHEETS, - perm: HAS_GSHEETS_INSTALLED, - }, - { - label: t('Upload CSV to database'), - name: 'Upload a CSV', - url: '/csvtodatabaseview/form', - perm: CSV_EXTENSIONS, - }, - { - label: t('Upload columnar file to database'), - name: 'Upload a Columnar file', - url: '/columnartodatabaseview/form', - perm: COLUMNAR_EXTENSIONS, - }, - { - label: t('Upload Excel file to database'), - name: 'Upload Excel', - url: '/exceltodatabaseview/form', - perm: EXCEL_EXTENSIONS, - }, - ], - }, - { - label: t('SQL query'), - url: '/superset/sqllab?new=true', - icon: 'fa-fw fa-search', - perm: 'can_sqllab', - view: 'Superset', - }, - { - label: t('Chart'), - url: '/chart/add', - icon: 'fa-fw fa-bar-chart', - perm: 'can_write', - view: 'Chart', - }, - { - label: t('Dashboard'), - url: '/dashboard/new', - icon: 'fa-fw fa-dashboard', - perm: 'can_write', - view: 'Dashboard', - }, - ]; - const menuIconAndLabel = (menu: MenuObjectProps) => ( <> {menu.label} ); - - const handleMenuSelection = (itemChose: any) => { - if (itemChose.key === GlobalMenuDataOptions.DB_CONNECTION) { - setShowModal(true); - } else if (itemChose.key === GlobalMenuDataOptions.GOOGLE_SHEETS) { - setShowModal(true); - setEngine('Google Sheets'); - } - }; - - const handleOnHideModal = () => { - setEngine(''); - setShowModal(false); - }; - return ( - - + {!navbarRight.user_is_anonymous && showActionDropdown && ( - {menu.childs.map((item, idx) => - typeof item !== 'string' && item.name && item.perm ? ( - <> - {idx === 2 && } - - {item.url ? ( - {item.label} - ) : ( - item.label - )} - - + {menu.childs.map(item => + typeof item !== 'string' && + item.name && + configMap[item.name] === true ? ( + + {item.label} + ) : null, )} diff --git a/superset-frontend/src/views/components/types.ts b/superset-frontend/src/views/components/types.ts deleted file mode 100644 index 0eff33390a727..0000000000000 --- a/superset-frontend/src/views/components/types.ts +++ /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 { NavBarProps, MenuObjectProps } from './Menu'; - -export interface ExtentionConfigs { - CSV_EXTENSIONS: boolean; - COLUMNAR_EXTENSIONS: boolean; - EXCEL_EXTENSIONS: boolean; - HAS_GSHEETS_INSTALLED: boolean; -} -export interface RightMenuProps { - align: 'flex-start' | 'flex-end'; - settings: MenuObjectProps[]; - navbarRight: NavBarProps; - isFrontendRoute: (path?: string) => boolean; -} - -export enum GlobalMenuDataOptions { - GOOGLE_SHEETS = 'gsheets', - DB_CONNECTION = 'dbconnection', -} diff --git a/superset/views/base.py b/superset/views/base.py index 067e8d74a727a..30acd51c8b6c0 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -62,8 +62,6 @@ from superset.commands.exceptions import CommandException, CommandInvalidError from superset.connectors.sqla import models from superset.datasets.commands.exceptions import get_dataset_exist_error_msg -from superset.db_engine_specs import get_available_engine_specs -from superset.db_engine_specs.gsheets import GSheetsEngineSpec from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import ( SupersetErrorException, @@ -368,10 +366,6 @@ def common_bootstrap_payload() -> Dict[str, Any]: ReportRecipientType.EMAIL, ] - # verify client has google sheets installed - available_specs = get_available_engine_specs() - frontend_config["HAS_GSHEETS_INSTALLED"] = bool(available_specs[GSheetsEngineSpec]) - bootstrap_data = { "flash_messages": messages, "conf": frontend_config,