From 05a1bb7793e4c3447a046373ab654217920bd487 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Wed, 21 Apr 2021 12:18:55 -0700 Subject: [PATCH 001/145] split db modal file --- .../views/CRUD/data/database/DatabaseList.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index 51808dffed04..d2400458541d 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -147,10 +147,13 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { ); } - function handleDatabaseEdit(database: DatabaseObject) { - // Set database and open modal + function handleDatabaseEditModal({ + database = null, + modalOpen = false, + }: { database?: DatabaseObject | null; modalOpen?: boolean } = {}) { + // Set database and modal setCurrentDatabase(database); - setDatabaseModalOpen(true); + setDatabaseModalOpen(modalOpen); } const canCreate = hasPerm('can_write'); @@ -176,8 +179,7 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { buttonStyle: 'primary', onClick: () => { // Ensure modal will be opened in add mode - setCurrentDatabase(null); - setDatabaseModalOpen(true); + handleDatabaseEditModal({ modalOpen: true }); }, }, ]; @@ -298,7 +300,8 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { }, { Cell: ({ row: { original } }: any) => { - const handleEdit = () => handleDatabaseEdit(original); + const handleEdit = () => + handleDatabaseEditModal({ database: original, modalOpen: true }); const handleDelete = () => openDatabaseDeleteModal(original); const handleExport = () => handleDatabaseExport(original); if (!canEdit && !canDelete && !canExport) { @@ -416,7 +419,7 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { setDatabaseModalOpen(false)} + onHide={handleDatabaseEditModal} onDatabaseAdd={() => { refreshData(); }} From a8afce998d03d968c535c2e43ff2196740a5fb1d Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Wed, 21 Apr 2021 12:18:55 -0700 Subject: [PATCH 002/145] split db modal file --- superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index d2400458541d..46522754ddbb 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -30,7 +30,7 @@ import Icons from 'src/components/Icons'; import ListView, { FilterOperator, Filters } from 'src/components/ListView'; import { commonMenuData } from 'src/views/CRUD/data/common'; import ImportModelsModal from 'src/components/ImportModal/index'; -import DatabaseModal from './DatabaseModal'; +import DatabaseModal from './databaseModal'; import { DatabaseObject } from './types'; const PAGE_SIZE = 25; From b69bf16a6e21e65872f351fa1231d880a13e49b2 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 27 Apr 2021 17:08:49 -0700 Subject: [PATCH 003/145] hook up available databases --- .../views/CRUD/data/database/DatabaseList.tsx | 2 +- .../DatabaseModal/DatabaseConnectionForm.tsx | 158 +++++++++++++ .../database/DatabaseModal/ExtraOptions.tsx | 11 +- .../database/DatabaseModal/SqlAlchemyForm.tsx | 8 +- .../database/DatabaseModal/index.test.jsx | 84 ++++++- .../data/database/DatabaseModal/index.tsx | 207 +++++++++++++++--- .../data/database/DatabaseModal/styles.ts | 140 +++++++++--- .../src/views/CRUD/data/database/types.ts | 48 ++++ superset-frontend/src/views/CRUD/hooks.ts | 18 +- superset/databases/schemas.py | 1 + 10 files changed, 596 insertions(+), 81 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index 46522754ddbb..d1105e706222 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -29,8 +29,8 @@ import { Tooltip } from 'src/components/Tooltip'; import Icons from 'src/components/Icons'; import ListView, { FilterOperator, Filters } from 'src/components/ListView'; import { commonMenuData } from 'src/views/CRUD/data/common'; +import DatabaseModal from 'src/views/CRUD/data/database/DatabaseModal'; import ImportModelsModal from 'src/components/ImportModal/index'; -import DatabaseModal from './databaseModal'; import { DatabaseObject } from './types'; const PAGE_SIZE = 25; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx new file mode 100644 index 000000000000..c8d98d73e0f8 --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx @@ -0,0 +1,158 @@ +/** + * 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, { FormEvent } from 'react'; +import cx from 'classnames'; +import { FormGroup, FormControl } from 'react-bootstrap'; +import FormLabel from 'src/components/Form/FormLabel'; +import { StyledFormHeader, formScrollableStyles } from './styles'; +import { DatabaseForm } from '../types'; + +export const FormFieldOrder = [ + 'host', + 'port', + 'database', + 'username', + 'password', + 'database_name', +]; + +const CHANGE_METHOD = { + onChange: 'onChange', + onPropertiesChange: 'onPropertiesChange', +}; + +const FORM_FIELD_MAP = { + host: { + description: 'Host', + type: 'text', + className: 'w-50', + placeholder: 'e.g. 127.0.0.1', + changeMethod: CHANGE_METHOD.onPropertiesChange, + }, + port: { + description: 'Port', + type: 'text', + className: 'w-50', + placeholder: 'e.g. 5432', + changeMethod: CHANGE_METHOD.onPropertiesChange, + }, + database: { + description: 'Database name', + type: 'text', + label: + 'Copy the name of the PostgreSQL database you are trying to connect to.', + placeholder: 'e.g. world_population', + changeMethod: CHANGE_METHOD.onPropertiesChange, + }, + username: { + description: 'Username', + type: 'text', + placeholder: 'e.g. Analytics', + changeMethod: CHANGE_METHOD.onPropertiesChange, + }, + password: { + description: 'Password', + type: 'text', + placeholder: 'e.g. ********', + changeMethod: CHANGE_METHOD.onPropertiesChange, + }, + database_name: { + description: 'Display Name', + type: 'text', + label: 'Pick a nickname for this database to display as in Superset.', + changeMethod: CHANGE_METHOD.onChange, + }, + query: { + additionalProperties: {}, + description: 'Additional parameters', + type: 'object', + changeMethod: CHANGE_METHOD.onPropertiesChange, + }, +}; + +const DatabaseConnectionForm = ({ + dbModel: { name, parameters }, + onParametersChange, + onChange, +}: { + dbModel: DatabaseForm; + onParametersChange: ( + event: FormEvent | { target: HTMLInputElement }, + ) => void; + onChange: ( + event: FormEvent | { target: HTMLInputElement }, + ) => void; +}) => ( + <> + +

Enter the required {name} credentials

+

+ Need help? Learn more about connecting to {name}. +

+
+
+ {parameters && + FormFieldOrder.filter( + (key: string) => + Object.keys(parameters.properties).includes(key) || + key === 'database_name', + ).map(field => { + const { + className, + description, + type, + placeholder, + label, + changeMethod, + } = FORM_FIELD_MAP[field]; + const onEdit = + changeMethod === CHANGE_METHOD.onChange + ? onChange + : onParametersChange; + return ( + + + {description} + + +

{label}

+
+ ); + })} +
+ +); + +export const FormFieldMap = FORM_FIELD_MAP; + +export default DatabaseConnectionForm; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx index 8f005f9411ea..e3987ad7ea7f 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx @@ -18,7 +18,7 @@ */ import React, { ChangeEvent, EventHandler } from 'react'; import cx from 'classnames'; -import { t } from '@superset-ui/core'; +import { t, SupersetTheme } from '@superset-ui/core'; import InfoTooltip from 'src/components/InfoTooltip'; import IndeterminateCheckbox from 'src/components/IndeterminateCheckbox'; import Collapse from 'src/components/Collapse'; @@ -26,7 +26,8 @@ import { StyledInputContainer, StyledJsonEditor, StyledExpandableForm, -} from 'src/views/CRUD/data/database/DatabaseModal/styles'; + antdCollapseStyles, +} from './styles'; import { DatabaseObject } from '../types'; const defaultExtra = @@ -48,7 +49,11 @@ const ExtraOptions = ({ const createAsOpen = !!(db?.allow_ctas || db?.allow_cvas); return ( - + antdCollapseStyles(theme)} + > diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx index 0bc2e0c816e0..d4c6278515ff 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx @@ -45,7 +45,7 @@ const SqlAlchemyTab = ({ type="text" name="database_name" value={db?.database_name || ''} - placeholder={t('Name your dataset')} + placeholder={t('Name your database')} onChange={onInputChange} /> @@ -71,15 +71,15 @@ const SqlAlchemyTab = ({ />
- {t('Refer to the ')} + {t('Refer to the')}{' '} {conf?.SQLALCHEMY_DISPLAY_TEXT ?? ''} - - {t(' for more information on how to structure your URI.')} + {' '} + {t('for more information on how to structure your URI.')}
+ + )} + ); }; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts index 38f5de7045cb..638b06b9d94d 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts @@ -17,8 +17,7 @@ * under the License. */ -import { styled } from '@superset-ui/core'; -import Modal from 'src/components/Modal'; +import { styled, css, SupersetTheme } from '@superset-ui/core'; import { JsonEditor } from 'src/components/AsyncAceEditor'; import Tabs from 'src/components/Tabs'; @@ -28,60 +27,126 @@ const EXPOSE_ALL_FORM_HEIGHT = EXPOSE_IN_SQLLAB_FORM_HEIGHT + 102; const anticonHeight = 12; -export const StyledModal = styled(Modal)` - .ant-collapse { - .ant-collapse-header { - padding-top: ${({ theme }) => theme.gridUnit * 3.5}px; - padding-bottom: ${({ theme }) => theme.gridUnit * 2.5}px; +export const StyledFormHeader = styled.header` + border-bottom: ${({ theme }) => `${theme.gridUnit * 0.25}px solid + ${theme.colors.grayscale.light2};`} + padding-left: ${({ theme }) => theme.gridUnit * 4}px; + padding-right: ${({ theme }) => theme.gridUnit * 4}px; + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + .helper { + color: ${({ theme }) => theme.colors.grayscale.base}; + font-size: ${({ theme }) => theme.typography.sizes.s - 1}px; + } + h4 { + color: ${({ theme }) => theme.colors.grayscale.dark2}; + font-weight: bold; + font-size: ${({ theme }) => theme.typography.sizes.l}px; + } +`; - .anticon.ant-collapse-arrow { - top: calc(50% - ${anticonHeight / 2}px); - } - .helper { - color: ${({ theme }) => theme.colors.grayscale.base}; - } - } - h4 { - font-size: 16px; - font-weight: bold; - margin-top: 0; - margin-bottom: ${({ theme }) => theme.gridUnit}px; +export const antdCollapseStyles = (theme: SupersetTheme) => css` + .ant-collapse-header { + padding-top: ${theme.gridUnit * 3.5}px; + padding-bottom: ${theme.gridUnit * 2.5}px; + + .anticon.ant-collapse-arrow { + top: calc(50% - ${anticonHeight / 2}px); } - p.helper { - margin-bottom: 0; - padding: 0; + .helper { + color: ${theme.colors.grayscale.base}; } } - .ant-modal-header { - padding: 18px 16px 16px; + h4 { + font-size: 16px; + font-weight: bold; + margin-top: 0; + margin-bottom: ${theme.gridUnit}px; + } + p.helper { + margin-bottom: 0; + padding: 0; + } +`; + +export const antDTabsStyles = css` + .ant-tabs-top > .ant-tabs-nav { + margin-bottom: 0; + } + .ant-tabs-tab { + margin-right: 0; } +`; + +export const antDModalNoPaddingStyles = css` .ant-modal-body { padding-left: 0; padding-right: 0; margin-bottom: 110px; } - .ant-tabs-top > .ant-tabs-nav { - margin-bottom: 0; +`; + +export const formScrollableStyles = (theme: SupersetTheme) => css` + overflow-y: scroll; + padding-left: ${theme.gridUnit * 4}px; + padding-right: ${theme.gridUnit * 4}px; +`; + +export const antDModalStyles = (theme: SupersetTheme) => css` + .ant-modal-header { + padding: ${theme.gridUnit * 4.5}px ${theme.gridUnit * 4}px + ${theme.gridUnit * 4}px; } + .ant-modal-close-x .close { - color: ${({ theme }) => theme.colors.grayscale.dark1}; + color: ${theme.colors.grayscale.dark1}; opacity: 1; } + .ant-modal-title > h4 { + font-weight: bold; + } +`; +export const formHelperStyles = (theme: SupersetTheme) => css` .required { - margin-left: ${({ theme }) => theme.gridUnit / 2}px; - color: ${({ theme }) => theme.colors.error.base}; + margin-left: ${theme.gridUnit / 2}px; + color: ${theme.colors.error.base}; } .helper { display: block; - padding: ${({ theme }) => theme.gridUnit}px 0; - color: ${({ theme }) => theme.colors.grayscale.light1}; - font-size: ${({ theme }) => theme.typography.sizes.s - 1}px; + padding: ${theme.gridUnit}px 0; + color: ${theme.colors.grayscale.light1}; + font-size: ${theme.typography.sizes.s - 1}px; text-align: left; } - .ant-modal-title > h4 { - font-weight: bold; +`; + +export const formStyles = (theme: SupersetTheme) => css` + .form-group { + margin-bottom: ${theme.gridUnit * 4}px; + &-w-50 { + display: inline-block; + width: ${`calc(50% - ${theme.gridUnit * 4}px)`}; + & + .form-group-w-50 { + margin-left: ${theme.gridUnit * 8}px; + } + } + .text-danger { + color: ${theme.colors.error.base}; + font-size: ${theme.typography.sizes.s - 1}px; + strong { + font-weight: normal; + } + } + } + .control-label { + color: ${theme.colors.grayscale.dark1}; + font-size: ${theme.typography.sizes.s - 1}px; + } + .helper { + color: ${theme.colors.grayscale.light1}; + font-size: ${theme.typography.sizes.s - 1}px; + margin-top: ${theme.gridUnit * 1.5}px; } .ant-alert { @@ -219,7 +284,12 @@ export const StyledExpandableForm = styled.div` export const StyledBasicTab = styled(Tabs.TabPane)` padding-left: ${({ theme }) => theme.gridUnit * 4}px; padding-right: ${({ theme }) => theme.gridUnit * 4}px; - margin-top: ${({ theme }) => theme.gridUnit * 4}px; + margin-top: ${({ theme }) => theme.gridUnit * 6}px; +`; + +export const buttonLinkStyles = css` + font-weight: 400; + text-transform: initial; `; export const EditHeader = styled.div` diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts index d0e6f5114d43..920f4b6881cf 100644 --- a/superset-frontend/src/views/CRUD/data/database/types.ts +++ b/superset-frontend/src/views/CRUD/data/database/types.ts @@ -30,6 +30,8 @@ export type DatabaseObject = { created_by?: null | DatabaseUser; changed_on_delta_humanized?: string; changed_on?: string; + parameters?: { database_name?: string; engine?: string }; + configuration_method: CONFIGURATION_METHOD; // Performance cache_timeout?: string; @@ -52,3 +54,49 @@ export type DatabaseObject = { allow_csv_upload?: boolean; extra?: string; }; + +export type DatabaseForm = { + engine: string; + name: string; + parameters: { + properties: { + database: { + description: string; + type: string; + }; + host: { + description: string; + type: string; + }; + password: { + description: string; + nullable: boolean; + type: string; + }; + port: { + description: string; + format: string; + type: string; + }; + query: { + additionalProperties: {}; + description: string; + type: string; + }; + username: { + description: string; + nullable: boolean; + type: string; + }; + }; + required: string[]; + type: string; + }; + preferred: boolean; + sqlalchemy_uri_placeholder: string; +}; + +export enum CONFIGURATION_METHOD { + SQLALCHEMY_URI = 'sqlalchemy_form', + DYNAMIC_FORM = 'dynamic_form', +} diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 9ec1104341cc..427549997caf 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -18,7 +18,7 @@ */ import rison from 'rison'; import { useState, useEffect, useCallback } from 'react'; -import { makeApi, SupersetClient, t } from '@superset-ui/core'; +import { makeApi, SupersetClient, t, JsonObject } from '@superset-ui/core'; import { createErrorHandler } from 'src/views/CRUD/utils'; import { FetchDataConfig } from 'src/components/ListView'; @@ -277,7 +277,7 @@ export function useSingleViewResource( .then( ({ json = {} }) => { updateState({ - resource: json.result, + resource: { id: json.id, ...json.result }, error: null, }); return json.id; @@ -643,3 +643,17 @@ export const testDatabaseConnection = ( }), ); }; + +export function useAvailableDatabases() { + const [availableDbs, setAvailableDbs] = useState(null); + + const getAvailable = useCallback(() => { + SupersetClient.get({ + endpoint: `/api/v1/database/available`, + }).then(({ json }) => { + setAvailableDbs(json); + }); + }, [setAvailableDbs]); + + return [availableDbs, getAvailable] as const; +} diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index a23659fc0fbf..924ac9496ed7 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -40,6 +40,7 @@ } database_name_description = "A database name to identify this connection." +port_description = "Port number for the database connection." cache_timeout_description = ( "Duration (in seconds) of the caching timeout for charts of this database. " "A timeout of 0 indicates that the cache never expires. " From 6eae68cb0e54c037eba336afd60fd9048d8fbc8d Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Mon, 17 May 2021 17:33:27 -0700 Subject: [PATCH 004/145] add comment --- .../DatabaseModal/DatabaseConnectionForm.tsx | 16 ++--- .../database/DatabaseModal/SqlAlchemyForm.tsx | 9 +-- .../data/database/DatabaseModal/index.tsx | 42 ++++++------ .../data/database/DatabaseModal/styles.ts | 64 +++++++++++-------- .../src/views/CRUD/data/database/types.ts | 2 + 5 files changed, 71 insertions(+), 62 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx index c8d98d73e0f8..f0542e481c12 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx @@ -18,8 +18,9 @@ */ import React, { FormEvent } from 'react'; import cx from 'classnames'; -import { FormGroup, FormControl } from 'react-bootstrap'; -import FormLabel from 'src/components/Form/FormLabel'; +import { InputProps } from 'antd/lib/input'; +import { FormLabel, FormItem } from 'src/components/Form'; +import { Input } from 'src/common/components'; import { StyledFormHeader, formScrollableStyles } from './styles'; import { DatabaseForm } from '../types'; @@ -93,10 +94,10 @@ const DatabaseConnectionForm = ({ }: { dbModel: DatabaseForm; onParametersChange: ( - event: FormEvent | { target: HTMLInputElement }, + event: FormEvent | { target: HTMLInputElement }, ) => void; onChange: ( - event: FormEvent | { target: HTMLInputElement }, + event: FormEvent | { target: HTMLInputElement }, ) => void; }) => ( <> @@ -126,7 +127,7 @@ const DatabaseConnectionForm = ({ ? onChange : onParametersChange; return ( - @@ -136,17 +137,16 @@ const DatabaseConnectionForm = ({ > {description} -

{label}

-
+ ); })} diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx index d4c6278515ff..cf442b4a57e2 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx @@ -17,9 +17,9 @@ * under the License. */ import React, { EventHandler, ChangeEvent, MouseEvent } from 'react'; -import { t, supersetTheme } from '@superset-ui/core'; +import { t, SupersetTheme } from '@superset-ui/core'; import Button from 'src/components/Button'; -import { StyledInputContainer } from './styles'; +import { StyledInputContainer, wideButton } from './styles'; import { DatabaseObject } from '../types'; @@ -86,10 +86,7 @@ const SqlAlchemyTab = ({ onClick={testConnection} cta buttonStyle="link" - style={{ - width: '100%', - border: `1px solid ${supersetTheme.colors.primary.base}`, - }} + css={(theme: SupersetTheme) => wideButton(theme)} > {t('Test connection')} diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index f47214c371f9..5cecee788006 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -45,22 +45,20 @@ import SqlAlchemyForm from './SqlAlchemyForm'; import DatabaseConnectionForm from './DatabaseConnectionForm'; import { - StyledBasicTab, - StyledModal, - EditHeader, - EditHeaderTitle, - EditHeaderSubtitle, + antDAlertStyles, + antDModalNoPaddingStyles, + antDModalStyles, + antDTabsStyles, + buttonLinkStyles, CreateHeader, CreateHeaderSubtitle, CreateHeaderTitle, - Divider, - StyledBasicTab, - antDModalStyles, - antDModalNoPaddingStyles, + EditHeader, + EditHeaderSubtitle, + EditHeaderTitle, formHelperStyles, formStyles, - antDTabsStyles, - buttonLinkStyles, + StyledBasicTab, } from './styles'; const DOCUMENTATION_LINK = @@ -76,14 +74,14 @@ interface DatabaseModalProps { } enum ActionType { - textChange, - inputChange, + configMethodChange, + dbSelected, editorChange, fetched, - reset, - dbSelected, + inputChange, parametersChange, - configMethodChange, + reset, + textChange, } interface DBReducerPayloadType { @@ -312,9 +310,7 @@ const DatabaseModal: FunctionComponent = ({ if (dbFetched) { setDB({ type: ActionType.fetched, - payload: { - ...dbFetched, - }, + payload: dbFetched, }); } }, [dbFetched]); @@ -381,11 +377,12 @@ const DatabaseModal: FunctionComponent = ({ )} - +
{t('Basic')}} key="1"> {useSqlAlchemyForm ? ( @@ -408,12 +405,13 @@ const DatabaseModal: FunctionComponent = ({ )} antDAlertStyles(theme)} message="Additional fields may be required" description={ <> Select databases require additional fields to be completed in - the next step to successfully connect the database. Learn what - requirements your databases has{' '} + the Advanced tab to successfully connect the database. Learn + what requirements your databases has{' '} css` } `; +export const antDAlertStyles = (theme: SupersetTheme) => css` + border: 1px solid ${theme.colors.info.base}; + padding: ${theme.gridUnit * 4}px; + margin: ${theme.gridUnit * 8}px 0 0; + .ant-alert-message { + color: ${theme.colors.info.dark2}; + font-size: ${theme.typography.sizes.s + 1}px; + font-weight: bold; + } + .ant-alert-description { + color: ${theme.colors.info.dark2}; + font-size: ${theme.typography.sizes.s + 1}px; + line-height: ${theme.gridUnit * 4}px; + .ant-alert-icon { + margin-right: ${theme.gridUnit * 2.5}px; + font-size: ${theme.typography.sizes.l + 1}px; + position: relative; + top: ${theme.gridUnit / 4}px; + } + } +`; + export const formHelperStyles = (theme: SupersetTheme) => css` .required { margin-left: ${theme.gridUnit / 2}px; @@ -121,6 +143,17 @@ export const formHelperStyles = (theme: SupersetTheme) => css` } `; +export const wideButton = (theme: SupersetTheme) => css` + width: 100%; + border: 1px solid ${theme.colors.primary.dark2}; + color: ${theme.colors.primary.dark2}; + &:hover, + &:focus { + border: 1px solid ${theme.colors.primary.dark1}; + color: ${theme.colors.primary.dark1}; + } +`; + export const formStyles = (theme: SupersetTheme) => css` .form-group { margin-bottom: ${theme.gridUnit * 4}px; @@ -148,21 +181,6 @@ export const formStyles = (theme: SupersetTheme) => css` font-size: ${theme.typography.sizes.s - 1}px; margin-top: ${theme.gridUnit * 1.5}px; } - - .ant-alert { - color: ${({ theme }) => theme.colors.info.dark2}; - border: 1px solid ${({ theme }) => theme.colors.info.base}; - font-size: ${({ theme }) => theme.gridUnit * 3}px; - padding: ${({ theme }) => theme.gridUnit * 4}px; - margin: ${({ theme }) => theme.gridUnit * 4}px 0 0; - } - .ant-alert-with-description { - .ant-alert-message, - .alert-with-description { - color: ${({ theme }) => theme.colors.info.dark2}; - font-weight: bold; - } - } .ant-modal-body { padding-top: 0; margin-bottom: 0; @@ -307,22 +325,20 @@ export const CreateHeader = styled.div` flex-direction: column; justify-content: center; padding: 0px; - margin: ${({ theme }) => theme.gridUnit * 4}px - ${({ theme }) => theme.gridUnit * 4}px - ${({ theme }) => theme.gridUnit * 9}px; + margin: 0 ${({ theme }) => theme.gridUnit * 4}px + ${({ theme }) => theme.gridUnit * 6}px; `; export const CreateHeaderTitle = styled.div` - color: ${({ theme }) => theme.colors.grayscale.dark1}; + color: ${({ theme }) => theme.colors.grayscale.dark2}; font-weight: bold; - font-size: ${({ theme }) => theme.typography.sizes.l}px; - padding: ${({ theme }) => theme.gridUnit * 1}px; + font-size: ${({ theme }) => theme.typography.sizes.m}px; + padding: ${({ theme }) => theme.gridUnit * 1}px 0; `; export const CreateHeaderSubtitle = styled.div` color: ${({ theme }) => theme.colors.grayscale.dark1}; font-size: ${({ theme }) => theme.typography.sizes.s}px; - padding: ${({ theme }) => theme.gridUnit * 1}px; `; export const EditHeaderTitle = styled.div` @@ -336,7 +352,3 @@ export const EditHeaderSubtitle = styled.div` font-size: ${({ theme }) => theme.typography.sizes.xl}px; font-weight: bold; `; - -export const Divider = styled.hr` - border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light1}; -`; diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts index 920f4b6881cf..2c386b5796a6 100644 --- a/superset-frontend/src/views/CRUD/data/database/types.ts +++ b/superset-frontend/src/views/CRUD/data/database/types.ts @@ -96,6 +96,8 @@ export type DatabaseForm = { sqlalchemy_uri_placeholder: string; }; +// the values should align with the database +// model enum in superset/superset/models/core.py export enum CONFIGURATION_METHOD { SQLALCHEMY_URI = 'sqlalchemy_form', DYNAMIC_FORM = 'dynamic_form', From e8cd4f4cab29ab7dfc9706c0dba6920f5115a513 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Wed, 21 Apr 2021 12:18:55 -0700 Subject: [PATCH 005/145] split db modal file --- .../src/views/CRUD/data/database/DatabaseList.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index d1105e706222..fda65baa2703 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -29,8 +29,9 @@ import { Tooltip } from 'src/components/Tooltip'; import Icons from 'src/components/Icons'; import ListView, { FilterOperator, Filters } from 'src/components/ListView'; import { commonMenuData } from 'src/views/CRUD/data/common'; -import DatabaseModal from 'src/views/CRUD/data/database/DatabaseModal'; import ImportModelsModal from 'src/components/ImportModal/index'; +import DatabaseModal from './DatabaseModal'; + import { DatabaseObject } from './types'; const PAGE_SIZE = 25; From 95917e40eb6d833ef783a59f80734b539cf4e5da Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 27 Apr 2021 17:08:49 -0700 Subject: [PATCH 006/145] hook up available databases --- .../database/DatabaseModal/index.test.jsx | 67 +++++++++++++++++++ .../data/database/DatabaseModal/index.tsx | 16 +++-- .../data/database/DatabaseModal/styles.ts | 4 -- 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index 1fc51e45a584..86967257dd26 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -319,5 +319,72 @@ describe('DatabaseModal', () => { const todoText = screen.getAllByText(/todo/i); expect(todoText[0]).toBeVisible(); }); + + describe('create database', () => { + it('should show a form when dynamic_form is selected', async () => { + const props = { + ...dbProps, + databaseId: null, + database_name: null, + sqlalchemy_uri: null, + }; + render(, { useRedux: true }); + // it should have the correct header text + const headerText = screen.getByText(/connect a database/i); + expect(headerText).toBeVisible(); + + await screen.findByText(/display name/i); + + // it does not fetch any databases if no id is passed in + expect(fetchMock.calls().length).toEqual(0); + + // todo we haven't hooked this up to load dynamically yet so + // we can't currently test it + }); + }); + + describe('edit database', () => { + it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { + render(, { useRedux: true }); + + // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + expect(tabs[0]).toHaveTextContent('Basic'); + expect(tabs[1]).toHaveTextContent('Advanced'); + + // it should have the correct header text + const headerText = screen.getByText(/edit database/i); + expect(headerText).toBeVisible(); + + // todo add more when this form is built out + }); + it('renders the dynamic form when the dynamic_form configuration method is set', async () => { + fetchMock.get(DATABASE_FETCH_ENDPOINT, { + result: { + id: 10, + database_name: 'my database', + expose_in_sqllab: false, + allow_ctas: false, + allow_cvas: false, + configuration_method: 'dynamic_form', + parameters: { + database: 'mydatabase', + }, + }, + }); + render(, { useRedux: true }); + + await screen.findByText(/todo/i); + + // // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + + // it should show a TODO for now + const todoText = screen.getAllByText(/todo/i); + expect(todoText[0]).toBeVisible(); + }); + }); }); }); diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 545cba00a7b1..57104b816dfb 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -248,14 +248,16 @@ const DatabaseModal: FunctionComponent = ({ // don't pass parameters if using the sqlalchemy uri delete update.parameters; } - updateResource(db.id as number, update as DatabaseObject).then(result => { - if (result) { - if (onDatabaseAdd) { - onDatabaseAdd(); - } - onClose(); + const result = await updateResource( + db.id as number, + update as DatabaseObject, + ); + if (result) { + if (onDatabaseAdd) { + onDatabaseAdd(); } - }); + onClose(); + } } else if (db) { // Create const dbId = await createResource(update as DatabaseObject); diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts index 8c33756962b0..4ae562996080 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts @@ -181,10 +181,6 @@ export const formStyles = (theme: SupersetTheme) => css` font-size: ${theme.typography.sizes.s - 1}px; margin-top: ${theme.gridUnit * 1.5}px; } - .ant-modal-body { - padding-top: 0; - margin-bottom: 0; - } .ant-tabs-content-holder { overflow: auto; max-height: 475px; From e7640724877caf356c114659f1d09e5e2691cf9f Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Fri, 21 May 2021 10:57:08 -0700 Subject: [PATCH 007/145] use new validation component --- .../integration/database/modal.test.ts | 24 ++ .../src/components/Form/FormItem.tsx | 4 + .../Form/LabeledErrorBoundInput.tsx | 18 +- .../DatabaseModal/DatabaseConnectionForm.tsx | 240 +++++++++++------- .../database/DatabaseModal/index.test.jsx | 2 +- .../data/database/DatabaseModal/index.tsx | 33 +-- .../data/database/DatabaseModal/styles.ts | 33 +-- .../src/views/CRUD/data/database/types.ts | 3 +- superset-frontend/src/views/CRUD/hooks.ts | 49 ++++ superset/databases/commands/validate.py | 6 +- superset/databases/schemas.py | 6 + superset/db_engine_specs/base.py | 6 +- 12 files changed, 284 insertions(+), 140 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts b/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts index e29ec48a9479..cf27d219324e 100644 --- a/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts @@ -88,4 +88,28 @@ describe('Add database', () => { // cy.wait(1000); // wait for potential (incorrect) closing annimation // cy.get('[data-test="database-modal"]').should('be.visible'); }); + + it('should close modal after save', () => { + cy.get('[data-test="btn-create-database"]').click(); + + // type values + cy.get('[data-test="database-modal"] input[name="database_name"]') + .focus() + .type('cypress'); + cy.get('[data-test="database-modal"] input[name="sqlalchemy_uri"]') + .focus() + .type('gsheets://'); + + // click save + cy.get('[data-test="modal-confirm-button"]:not(:disabled)').click(); + + // should show error alerts and keep modal open + cy.get('.toast').contains('error'); + cy.wait(1000); // wait for potential (incorrect) closing annimation + cy.get('[data-test="database-modal"]').should('be.visible'); + + // should be able to close modal + cy.get('[data-test="modal-cancel-button"]').click(); + cy.get('[data-test="database-modal"]').should('not.be.visible'); + }); }); diff --git a/superset-frontend/src/components/Form/FormItem.tsx b/superset-frontend/src/components/Form/FormItem.tsx index ab301a883e54..2731a215faf1 100644 --- a/superset-frontend/src/components/Form/FormItem.tsx +++ b/superset-frontend/src/components/Form/FormItem.tsx @@ -41,6 +41,10 @@ const StyledItem = styled(Form.Item)` } } } + .ant-form-item-explain { + color: ${theme.colors.grayscale.light1}; + font-size: ${theme.typography.sizes.s - 1}px; + } `} `; diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx index 8569b554a020..75df2bb088cb 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx @@ -25,17 +25,18 @@ import FormLabel from './FormLabel'; export interface LabeledErrorBoundInputProps { label?: string; validationMethods: - | { onBlur: (value: any) => string } - | { onChange: (value: any) => string }; + | { onBlur: (value: any) => void } + | { onChange: (value: any) => void }; errorMessage: string | null; helpText?: string; required?: boolean; id?: string; + classname?: string; [x: string]: any; } const StyledInput = styled(Input)` - margin: 8px 0; + margin: ${({ theme }) => `${theme.gridUnit}px 0 ${theme.gridUnit * 2}px`}; `; const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css` @@ -60,6 +61,12 @@ const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css` } }`} `; +const StyledFormGroup = styled('div')` + margin-bottom: ${({ theme }) => theme.gridUnit * 5}px; + .ant-form-item { + margin-bottom: 0; + } +`; const LabeledErrorBoundInput = ({ label, @@ -68,9 +75,10 @@ const LabeledErrorBoundInput = ({ helpText, required = false, id, + className, ...props }: LabeledErrorBoundInputProps) => ( - <> + {label} @@ -83,7 +91,7 @@ const LabeledErrorBoundInput = ({ > - + ); export default LabeledErrorBoundInput; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx index f0542e481c12..5c7c729da589 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx @@ -17,11 +17,14 @@ * under the License. */ import React, { FormEvent } from 'react'; -import cx from 'classnames'; +import { SupersetTheme, JsonObject } from '@superset-ui/core'; import { InputProps } from 'antd/lib/input'; -import { FormLabel, FormItem } from 'src/components/Form'; -import { Input } from 'src/common/components'; -import { StyledFormHeader, formScrollableStyles } from './styles'; +import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput'; +import { + StyledFormHeader, + formScrollableStyles, + validatedFormStyles, +} from './styles'; import { DatabaseForm } from '../types'; export const FormFieldOrder = [ @@ -33,64 +36,137 @@ export const FormFieldOrder = [ 'database_name', ]; -const CHANGE_METHOD = { - onChange: 'onChange', - onPropertiesChange: 'onPropertiesChange', -}; +interface FieldPropTypes { + required: boolean; + changeMethods: { onParametersChange: (value: any) => string } & { + onChange: (value: any) => string; + }; + validationErrors: JsonObject | null; + getValidation: () => void; +} + +const hostField = ({ + required, + changeMethods, + getValidation, + validationErrors, +}: FieldPropTypes) => ( + +); +const portField = ({ + required, + changeMethods, + getValidation, + validationErrors, +}: FieldPropTypes) => ( + +); +const databaseField = ({ + required, + changeMethods, + getValidation, + validationErrors, +}: FieldPropTypes) => ( + +); +const usernameField = ({ + required, + changeMethods, + getValidation, + validationErrors, +}: FieldPropTypes) => ( + +); +const passwordField = ({ + required, + changeMethods, + getValidation, + validationErrors, +}: FieldPropTypes) => ( + +); +const displayField = ({ + required, + changeMethods, + getValidation, + validationErrors, +}: FieldPropTypes) => ( + +); const FORM_FIELD_MAP = { - host: { - description: 'Host', - type: 'text', - className: 'w-50', - placeholder: 'e.g. 127.0.0.1', - changeMethod: CHANGE_METHOD.onPropertiesChange, - }, - port: { - description: 'Port', - type: 'text', - className: 'w-50', - placeholder: 'e.g. 5432', - changeMethod: CHANGE_METHOD.onPropertiesChange, - }, - database: { - description: 'Database name', - type: 'text', - label: - 'Copy the name of the PostgreSQL database you are trying to connect to.', - placeholder: 'e.g. world_population', - changeMethod: CHANGE_METHOD.onPropertiesChange, - }, - username: { - description: 'Username', - type: 'text', - placeholder: 'e.g. Analytics', - changeMethod: CHANGE_METHOD.onPropertiesChange, - }, - password: { - description: 'Password', - type: 'text', - placeholder: 'e.g. ********', - changeMethod: CHANGE_METHOD.onPropertiesChange, - }, - database_name: { - description: 'Display Name', - type: 'text', - label: 'Pick a nickname for this database to display as in Superset.', - changeMethod: CHANGE_METHOD.onChange, - }, - query: { - additionalProperties: {}, - description: 'Additional parameters', - type: 'object', - changeMethod: CHANGE_METHOD.onPropertiesChange, - }, + host: hostField, + port: portField, + database: databaseField, + username: usernameField, + password: passwordField, + database_name: displayField, }; const DatabaseConnectionForm = ({ dbModel: { name, parameters }, onParametersChange, onChange, + validationErrors, + getValidation, }: { dbModel: DatabaseForm; onParametersChange: ( @@ -99,6 +175,8 @@ const DatabaseConnectionForm = ({ onChange: ( event: FormEvent | { target: HTMLInputElement }, ) => void; + validationErrors: JsonObject | null; + getValidation: () => void; }) => ( <> @@ -107,52 +185,30 @@ const DatabaseConnectionForm = ({ Need help? Learn more about connecting to {name}.

-
+
[ + formScrollableStyles, + validatedFormStyles(theme), + ]} + > {parameters && FormFieldOrder.filter( (key: string) => Object.keys(parameters.properties).includes(key) || key === 'database_name', - ).map(field => { - const { - className, - description, - type, - placeholder, - label, - changeMethod, - } = FORM_FIELD_MAP[field]; - const onEdit = - changeMethod === CHANGE_METHOD.onChange - ? onChange - : onParametersChange; - return ( - - - {description} - - -

{label}

-
- ); - })} + ).map(field => + FORM_FIELD_MAP[field]({ + required: parameters.required.includes(field), + changeMethods: { onParametersChange, onChange }, + validationErrors, + getValidation, + key: field, + }), + )}
); - export const FormFieldMap = FORM_FIELD_MAP; export default DatabaseConnectionForm; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index 86967257dd26..3f10914cda0a 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -336,7 +336,7 @@ describe('DatabaseModal', () => { await screen.findByText(/display name/i); // it does not fetch any databases if no id is passed in - expect(fetchMock.calls().length).toEqual(0); + expect(fetchMock.calls(DATABASE_FETCH_ENDPOINT).length).toEqual(0); // todo we haven't hooked this up to load dynamically yet so // we can't currently test it diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 57104b816dfb..265a8561923c 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -33,6 +33,7 @@ import { testDatabaseConnection, useSingleViewResource, useAvailableDatabases, + useDatabaseValidation, } from 'src/views/CRUD/hooks'; import { useCommonConf } from 'src/views/CRUD/data/database/state'; import { @@ -50,10 +51,9 @@ import { antDModalStyles, antDTabsStyles, buttonLinkStyles, - CreateHeader, + TabHeader, CreateHeaderSubtitle, CreateHeaderTitle, - EditHeader, EditHeaderSubtitle, EditHeaderTitle, formHelperStyles, @@ -109,7 +109,7 @@ type DBReducerActionType = | { type: ActionType.dbSelected; payload: { - parameters: { engine?: string }; + engine?: string; configuration_method: CONFIGURATION_METHOD; }; } @@ -127,8 +127,6 @@ function dbReducer( ): Partial | null { const trimmedState = { ...(state || {}), - database_name: state?.database_name?.trim() || '', - sqlalchemy_uri: state?.sqlalchemy_uri || '', }; switch (action.type) { @@ -163,9 +161,7 @@ function dbReducer( }; case ActionType.fetched: return { - parameters: { - engine: trimmedState.parameters?.engine, - }, + engine: trimmedState.engine, configuration_method: trimmedState.configuration_method, ...action.payload, }; @@ -196,13 +192,16 @@ const DatabaseModal: FunctionComponent = ({ >(dbReducer, null); const [tabKey, setTabKey] = useState(DEFAULT_TAB_KEY); const [availableDbs, getAvailableDbs] = useAvailableDatabases(); + const [validationErrors, getValidation] = useDatabaseValidation(); const [hasConnectedDb, setHasConnectedDb] = useState(false); + const [dbName, setDbName] = useState(''); const conf = useCommonConf(); const isEditMode = !!databaseId; const useSqlAlchemyForm = db?.configuration_method === CONFIGURATION_METHOD.SQLALCHEMY_URI; const useTabLayout = isEditMode || useSqlAlchemyForm; + // Database fetch logic const { state: { loading: dbLoading, resource: dbFetched }, @@ -302,7 +301,6 @@ const DatabaseModal: FunctionComponent = ({ setDB({ type: ActionType.dbSelected, payload: { - parameters: {}, configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI, }, // todo hook this up to step 1 }); @@ -318,6 +316,9 @@ const DatabaseModal: FunctionComponent = ({ type: ActionType.fetched, payload: dbFetched, }); + // keep a copy of the name separate for display purposes + // because it shouldn't change when the form is updated + setDbName(dbFetched.database_name); } }, [dbFetched]); @@ -328,7 +329,7 @@ const DatabaseModal: FunctionComponent = ({ const dbModel: DatabaseForm = availableDbs?.databases?.find( (available: { engine: string | undefined }) => - available.engine === db?.parameters?.engine, + available.engine === db?.engine, ) || {}; const disableSave = @@ -364,12 +365,12 @@ const DatabaseModal: FunctionComponent = ({ } > {isEditMode ? ( - + {db?.backend} - {db?.database_name} - + {dbName} + ) : ( - + Enter Primary Credentials Need help? Learn how to connect your database{' '} @@ -382,7 +383,7 @@ const DatabaseModal: FunctionComponent = ({
. - + )}
= ({ value: target.value, }) } + getValidation={() => getValidation(db)} + validationErrors={validationErrors} />
); - export const FormFieldMap = FORM_FIELD_MAP; export default DatabaseConnectionForm; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index 1fc51e45a584..86967257dd26 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -319,5 +319,72 @@ describe('DatabaseModal', () => { const todoText = screen.getAllByText(/todo/i); expect(todoText[0]).toBeVisible(); }); + + describe('create database', () => { + it('should show a form when dynamic_form is selected', async () => { + const props = { + ...dbProps, + databaseId: null, + database_name: null, + sqlalchemy_uri: null, + }; + render(, { useRedux: true }); + // it should have the correct header text + const headerText = screen.getByText(/connect a database/i); + expect(headerText).toBeVisible(); + + await screen.findByText(/display name/i); + + // it does not fetch any databases if no id is passed in + expect(fetchMock.calls().length).toEqual(0); + + // todo we haven't hooked this up to load dynamically yet so + // we can't currently test it + }); + }); + + describe('edit database', () => { + it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { + render(, { useRedux: true }); + + // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + expect(tabs[0]).toHaveTextContent('Basic'); + expect(tabs[1]).toHaveTextContent('Advanced'); + + // it should have the correct header text + const headerText = screen.getByText(/edit database/i); + expect(headerText).toBeVisible(); + + // todo add more when this form is built out + }); + it('renders the dynamic form when the dynamic_form configuration method is set', async () => { + fetchMock.get(DATABASE_FETCH_ENDPOINT, { + result: { + id: 10, + database_name: 'my database', + expose_in_sqllab: false, + allow_ctas: false, + allow_cvas: false, + configuration_method: 'dynamic_form', + parameters: { + database: 'mydatabase', + }, + }, + }); + render(, { useRedux: true }); + + await screen.findByText(/todo/i); + + // // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + + // it should show a TODO for now + const todoText = screen.getAllByText(/todo/i); + expect(todoText[0]).toBeVisible(); + }); + }); }); }); diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 545cba00a7b1..265a8561923c 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -33,6 +33,7 @@ import { testDatabaseConnection, useSingleViewResource, useAvailableDatabases, + useDatabaseValidation, } from 'src/views/CRUD/hooks'; import { useCommonConf } from 'src/views/CRUD/data/database/state'; import { @@ -50,10 +51,9 @@ import { antDModalStyles, antDTabsStyles, buttonLinkStyles, - CreateHeader, + TabHeader, CreateHeaderSubtitle, CreateHeaderTitle, - EditHeader, EditHeaderSubtitle, EditHeaderTitle, formHelperStyles, @@ -109,7 +109,7 @@ type DBReducerActionType = | { type: ActionType.dbSelected; payload: { - parameters: { engine?: string }; + engine?: string; configuration_method: CONFIGURATION_METHOD; }; } @@ -127,8 +127,6 @@ function dbReducer( ): Partial | null { const trimmedState = { ...(state || {}), - database_name: state?.database_name?.trim() || '', - sqlalchemy_uri: state?.sqlalchemy_uri || '', }; switch (action.type) { @@ -163,9 +161,7 @@ function dbReducer( }; case ActionType.fetched: return { - parameters: { - engine: trimmedState.parameters?.engine, - }, + engine: trimmedState.engine, configuration_method: trimmedState.configuration_method, ...action.payload, }; @@ -196,13 +192,16 @@ const DatabaseModal: FunctionComponent = ({ >(dbReducer, null); const [tabKey, setTabKey] = useState(DEFAULT_TAB_KEY); const [availableDbs, getAvailableDbs] = useAvailableDatabases(); + const [validationErrors, getValidation] = useDatabaseValidation(); const [hasConnectedDb, setHasConnectedDb] = useState(false); + const [dbName, setDbName] = useState(''); const conf = useCommonConf(); const isEditMode = !!databaseId; const useSqlAlchemyForm = db?.configuration_method === CONFIGURATION_METHOD.SQLALCHEMY_URI; const useTabLayout = isEditMode || useSqlAlchemyForm; + // Database fetch logic const { state: { loading: dbLoading, resource: dbFetched }, @@ -248,14 +247,16 @@ const DatabaseModal: FunctionComponent = ({ // don't pass parameters if using the sqlalchemy uri delete update.parameters; } - updateResource(db.id as number, update as DatabaseObject).then(result => { - if (result) { - if (onDatabaseAdd) { - onDatabaseAdd(); - } - onClose(); + const result = await updateResource( + db.id as number, + update as DatabaseObject, + ); + if (result) { + if (onDatabaseAdd) { + onDatabaseAdd(); } - }); + onClose(); + } } else if (db) { // Create const dbId = await createResource(update as DatabaseObject); @@ -300,7 +301,6 @@ const DatabaseModal: FunctionComponent = ({ setDB({ type: ActionType.dbSelected, payload: { - parameters: {}, configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI, }, // todo hook this up to step 1 }); @@ -316,6 +316,9 @@ const DatabaseModal: FunctionComponent = ({ type: ActionType.fetched, payload: dbFetched, }); + // keep a copy of the name separate for display purposes + // because it shouldn't change when the form is updated + setDbName(dbFetched.database_name); } }, [dbFetched]); @@ -326,7 +329,7 @@ const DatabaseModal: FunctionComponent = ({ const dbModel: DatabaseForm = availableDbs?.databases?.find( (available: { engine: string | undefined }) => - available.engine === db?.parameters?.engine, + available.engine === db?.engine, ) || {}; const disableSave = @@ -362,12 +365,12 @@ const DatabaseModal: FunctionComponent = ({ } > {isEditMode ? ( - + {db?.backend} - {db?.database_name} - + {dbName} + ) : ( - + Enter Primary Credentials Need help? Learn how to connect your database{' '} @@ -380,7 +383,7 @@ const DatabaseModal: FunctionComponent = ({ . - + )}
= ({ value: target.value, }) } + getValidation={() => getValidation(db)} + validationErrors={validationErrors} /> , + , + ]; + useEffect(() => { if (show) { setTabKey(DEFAULT_TAB_KEY); @@ -423,7 +432,7 @@ const DatabaseModal: FunctionComponent = ({ {dbName} )} - {/* Add styled header here when not in edit mode*/} + {/* Add styled header here when not in edit mode */}
= ({ width="500px" show={show} title={

{t('Connect a database')}

} + footer={renderModalFooter()} > {hasConnectedDb ? ( Date: Wed, 2 Jun 2021 16:14:36 -0400 Subject: [PATCH 035/145] add finsh buttong --- .../data/database/DatabaseModal/index.tsx | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 487817c8a3aa..3d23a56f59c2 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -346,14 +346,26 @@ const DatabaseModal: FunctionComponent = ({ ); - const renderModalFooter = () => [ - , - , - ]; + const renderModalFooter = () => + db // if db show back + connect + ? [ + , + !hasConnectedDb ? ( // if hasConnectedDb show back + finish + + ) : ( + + ), + ] + : null; useEffect(() => { if (show) { From 530150e948a3299fea5576195e18caa58d509e3a Mon Sep 17 00:00:00 2001 From: hughhhh Date: Wed, 2 Jun 2021 16:42:59 -0400 Subject: [PATCH 036/145] refactor db modal render --- .../data/database/DatabaseModal/index.tsx | 82 +++++++++++-------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 3d23a56f59c2..b404e8500941 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -537,7 +537,7 @@ const DatabaseModal: FunctionComponent = ({ title={

{t('Connect a database')}

} footer={renderModalFooter()} > - {hasConnectedDb ? ( + {hasConnectedDb ? ( {/* Step 3 */} @@ -557,28 +557,9 @@ const DatabaseModal: FunctionComponent = ({ onEditorChange={(payload: { name: string; json: any }) => onChange(ActionType.editorChange, payload) } - /> + /> {/* Step 3 */} ) : ( <> - - onChange(ActionType.parametersChange, { - type: target.type, - name: target.name, - checked: target.checked, - value: target.value, - }) - } - onChange={({ target }: { target: HTMLInputElement }) => - onChange(ActionType.textChange, { - name: target.name, - value: target.value, - }) - } - getValidation={() => getValidation(db)} - validationErrors={validationErrors} - /> {/* Step 1 */} {!isLoading && !db && ( @@ -587,20 +568,51 @@ const DatabaseModal: FunctionComponent = ({ )} {/* Step 1 */} - + + {/* Step 2 */} + {!isLoading && db && ( + <> + + onChange(ActionType.parametersChange, { + type: target.type, + name: target.name, + checked: target.checked, + value: target.value, + }) + } + onChange={({ target }: { target: HTMLInputElement }) => + onChange(ActionType.textChange, { + name: target.name, + value: target.value, + }) + } + getValidation={() => getValidation(db)} + validationErrors={validationErrors} + /> + + + {/* Step 2 */} + + )} )} From 508a6b9eb2d1a1ea2060fcde73272befbda6c373 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Wed, 2 Jun 2021 16:45:21 -0400 Subject: [PATCH 037/145] fix comments issue --- .../src/views/CRUD/data/database/DatabaseModal/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index b404e8500941..3819dd760317 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -537,7 +537,7 @@ const DatabaseModal: FunctionComponent = ({ title={

{t('Connect a database')}

} footer={renderModalFooter()} > - {hasConnectedDb ? ( {/* Step 3 */} + {hasConnectedDb ? ( @@ -557,7 +557,7 @@ const DatabaseModal: FunctionComponent = ({ onEditorChange={(payload: { name: string; json: any }) => onChange(ActionType.editorChange, payload) } - /> {/* Step 3 */} + /> ) : ( <> {/* Step 1 */} From 47c30cb8b3f0467a37b5a3b7c41b2f07336252e0 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 2 Jun 2021 17:07:49 -0400 Subject: [PATCH 038/145] added engine to model --- .../views/CRUD/data/database/DatabaseModal/index.tsx | 2 +- .../src/views/CRUD/data/database/types.ts | 1 - superset/databases/api.py | 1 + superset/models/core.py | 10 +++++----- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 25ead2b9b065..a017438a0902 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -331,7 +331,7 @@ const DatabaseModal: FunctionComponent = ({ availableDbs?.databases?.find( (available: { engine: string | undefined }) => // TODO: we need a centralized engine in one place - available.engine === db?.engine || db?.backend, + available.engine === db?.engine, ) || {}; const disableSave = !hasConnectedDb && diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts index d8c2a0e020a0..fd6307ca5e38 100644 --- a/superset-frontend/src/views/CRUD/data/database/types.ts +++ b/superset-frontend/src/views/CRUD/data/database/types.ts @@ -34,7 +34,6 @@ export type DatabaseObject = { database_name?: string; host?: string; port?: number; - engine?: string; database?: string; username?: string; password?: string; diff --git a/superset/databases/api.py b/superset/databases/api.py index d64238baf4a3..4b903d07f20f 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -107,6 +107,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "allow_cvas", "allow_dml", "backend", + "engine", "force_ctas_schema", "allow_multi_schema_metadata_fetch", "impersonate_user", diff --git a/superset/models/core.py b/superset/models/core.py index ee8e822477a8..3264fb1c908a 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -215,6 +215,7 @@ def data(self) -> Dict[str, Any]: "id": self.id, "name": self.database_name, "backend": self.backend, + "engine": self.backend, "configuration_method": self.configuration_method, "allow_multi_schema_metadata_fetch": self.allow_multi_schema_metadata_fetch, "allows_subquery": self.allows_subquery, @@ -237,19 +238,18 @@ def backend(self) -> str: sqlalchemy_url = make_url(self.sqlalchemy_uri_decrypted) return sqlalchemy_url.get_backend_name() # pylint: disable=no-member + engine = backend + @property def parameters(self) -> Dict[str, Any]: - # Build parameters if db_engine_spec is a subclass of BasicParametersMixin - parameters = {"engine": self.backend} - if hasattr(self.db_engine_spec, "parameters_schema") and hasattr( self.db_engine_spec, "get_parameters_from_uri" ): uri = make_url(self.sqlalchemy_uri_decrypted) encrypted_extra = self.get_encrypted_extra() - return {**parameters, **self.db_engine_spec.get_parameters_from_uri(uri, encrypted_extra=encrypted_extra)} # type: ignore + return self.db_engine_spec.get_parameters_from_uri(uri, encrypted_extra=encrypted_extra) # type: ignore - return parameters + return {} @property def metadata_cache_timeout(self) -> Dict[str, Any]: From 9fac1ebfb2bbaf4ecedad0ca9f3044fcb7adde76 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 2 Jun 2021 17:44:01 -0400 Subject: [PATCH 039/145] elizabeth revisions --- .../integration/database/modal.test.ts | 118 +++++++++--------- .../database/DatabaseModal/index.test.jsx | 2 - .../data/database/DatabaseModal/index.tsx | 2 +- superset/databases/api.py | 1 - superset/models/core.py | 3 - 5 files changed, 60 insertions(+), 66 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts b/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts index a0125c543c64..e54a600ecacf 100644 --- a/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts @@ -16,76 +16,76 @@ * specific language governing permissions and limitations * under the License. */ -// import { DATABASE_LIST } from './helper'; +import { DATABASE_LIST } from './helper'; // TODO: Add new tests with the modal -// describe('Add database', () => { -// beforeEach(() => { -// cy.login(); -// }); +describe('Add database', () => { + beforeEach(() => { + cy.login(); + }); -// it('should keep create modal open when error', () => { -// cy.visit(DATABASE_LIST); + xit('should keep create modal open when error', () => { + cy.visit(DATABASE_LIST); -// // open modal -// cy.get('[data-test="btn-create-database"]').click(); + // open modal + cy.get('[data-test="btn-create-database"]').click(); -// // values should be blank -// cy.get('[data-test="database-modal"] input[name="database_name"]').should( -// 'have.value', -// '', -// ); -// cy.get('[data-test="database-modal"] input[name="sqlalchemy_uri"]').should( -// 'have.value', -// '', -// ); + // values should be blank + cy.get('[data-test="database-modal"] input[name="database_name"]').should( + 'have.value', + '', + ); + cy.get('[data-test="database-modal"] input[name="sqlalchemy_uri"]').should( + 'have.value', + '', + ); -// // type values -// cy.get('[data-test="database-modal"] input[name="database_name"]') -// .focus() -// .type('cypress'); -// cy.get('[data-test="database-modal"] input[name="sqlalchemy_uri"]') -// .focus() -// .type('bad_db_uri'); + // type values + cy.get('[data-test="database-modal"] input[name="database_name"]') + .focus() + .type('cypress'); + cy.get('[data-test="database-modal"] input[name="sqlalchemy_uri"]') + .focus() + .type('bad_db_uri'); -// // click save -// cy.get('[data-test="modal-confirm-button"]:not(:disabled)').click(); + // click save + cy.get('[data-test="modal-confirm-button"]:not(:disabled)').click(); -// // should show error alerts and keep modal open -// cy.get('.toast').contains('error'); -// cy.wait(1000); // wait for potential (incorrect) closing annimation -// cy.get('[data-test="database-modal"]').should('be.visible'); + // should show error alerts and keep modal open + cy.get('.toast').contains('error'); + cy.wait(1000); // wait for potential (incorrect) closing annimation + cy.get('[data-test="database-modal"]').should('be.visible'); -// // should be able to close modal -// cy.get('[data-test="modal-cancel-button"]').click(); -// cy.get('[data-test="database-modal"]').should('not.be.visible'); -// }); + // should be able to close modal + cy.get('[data-test="modal-cancel-button"]').click(); + cy.get('[data-test="database-modal"]').should('not.be.visible'); + }); -// it('should keep update modal open when error', () => { -// // open modal -// cy.get('[data-test="database-edit"]:last').click(); + xit('should keep update modal open when error', () => { + // open modal + cy.get('[data-test="database-edit"]:last').click(); -// // it should show saved values -// cy.get('[data-test="database-modal"]:last input[name="sqlalchemy_uri"]') -// .invoke('val') -// .should('not.be.empty'); -// cy.get('[data-test="database-modal"] input[name="database_name"]') -// .invoke('val') -// .should('not.be.empty'); + // it should show saved values + cy.get('[data-test="database-modal"]:last input[name="sqlalchemy_uri"]') + .invoke('val') + .should('not.be.empty'); + cy.get('[data-test="database-modal"] input[name="database_name"]') + .invoke('val') + .should('not.be.empty'); -// cy.get('[data-test="database-modal"]:last input[name="sqlalchemy_uri"]') -// .focus() -// .dblclick() -// .type('{selectall}{backspace}bad_uri'); + cy.get('[data-test="database-modal"]:last input[name="sqlalchemy_uri"]') + .focus() + .dblclick() + .type('{selectall}{backspace}bad_uri'); -// // click save -// cy.get('[data-test="modal-confirm-button"]:not(:disabled)').click(); + // click save + cy.get('[data-test="modal-confirm-button"]:not(:disabled)').click(); -// // should show error alerts -// // TODO(hugh): Update this test -// // cy.get('.toast').contains('error').should('be.visible'); + // should show error alerts + // TODO(hugh): Update this test + // cy.get('.toast').contains('error').should('be.visible'); -// // modal should still be open -// // cy.wait(1000); // wait for potential (incorrect) closing annimation -// // cy.get('[data-test="database-modal"]').should('be.visible'); -// }); -// }); + // modal should still be open + // cy.wait(1000); // wait for potential (incorrect) closing annimation + // cy.get('[data-test="database-modal"]').should('be.visible'); + }); +}); diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index bd43339b517c..760c2134edec 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -290,8 +290,6 @@ describe('DatabaseModal', () => { // it should have the correct header text const headerText = screen.getByText(/edit database/i); expect(headerText).toBeVisible(); - - // todo add more when this form is built out }); it('renders the dynamic form when the dynamic_form configuration method is set', async () => { fetchMock.get(DATABASE_FETCH_ENDPOINT, { diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index a017438a0902..25ead2b9b065 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -331,7 +331,7 @@ const DatabaseModal: FunctionComponent = ({ availableDbs?.databases?.find( (available: { engine: string | undefined }) => // TODO: we need a centralized engine in one place - available.engine === db?.engine, + available.engine === db?.engine || db?.backend, ) || {}; const disableSave = !hasConnectedDb && diff --git a/superset/databases/api.py b/superset/databases/api.py index 4b903d07f20f..d64238baf4a3 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -107,7 +107,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "allow_cvas", "allow_dml", "backend", - "engine", "force_ctas_schema", "allow_multi_schema_metadata_fetch", "impersonate_user", diff --git a/superset/models/core.py b/superset/models/core.py index 3264fb1c908a..6fe61cbfe985 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -215,7 +215,6 @@ def data(self) -> Dict[str, Any]: "id": self.id, "name": self.database_name, "backend": self.backend, - "engine": self.backend, "configuration_method": self.configuration_method, "allow_multi_schema_metadata_fetch": self.allow_multi_schema_metadata_fetch, "allows_subquery": self.allows_subquery, @@ -238,8 +237,6 @@ def backend(self) -> str: sqlalchemy_url = make_url(self.sqlalchemy_uri_decrypted) return sqlalchemy_url.get_backend_name() # pylint: disable=no-member - engine = backend - @property def parameters(self) -> Dict[str, Any]: if hasattr(self.db_engine_spec, "parameters_schema") and hasattr( From 607e01a9f5c634bc2101908cc648ebf032052746 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Thu, 3 Jun 2021 12:17:43 -0400 Subject: [PATCH 040/145] add header --- .../src/views/CRUD/data/database/DatabaseModal/index.tsx | 7 +++++++ .../src/views/CRUD/data/database/DatabaseModal/styles.ts | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 3819dd760317..c917487ee2ac 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -61,6 +61,7 @@ import { formStyles, StyledBasicTab, SelectDatabaseStyles, + StyledFormHeader, } from './styles'; const DOCUMENTATION_LINK = @@ -563,6 +564,12 @@ const DatabaseModal: FunctionComponent = ({ {/* Step 1 */} {!isLoading && !db && ( + +
+

Step 1 of 3

+

Select a database to connect

+
+
{renderPreferredSelector()} {renderAvailableSelector()}
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts index 9c1a1f2f6ea4..fdd49cf3cf11 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts @@ -42,6 +42,15 @@ export const StyledFormHeader = styled.header` font-weight: bold; font-size: ${({ theme }) => theme.typography.sizes.l}px; } + + .select-db { + .helper { + margin-top: 0; + } + h4 { + margin: 0 0 29px; + } + } `; export const antdCollapseStyles = (theme: SupersetTheme) => css` From b343ca4ca78536712a34c0e69573f2e7d342451e Mon Sep 17 00:00:00 2001 From: hughhhh Date: Thu, 3 Jun 2021 12:36:51 -0400 Subject: [PATCH 041/145] add bottom footer to sqlalchemy form --- .../src/views/CRUD/data/database/DatabaseModal/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index c917487ee2ac..e121416fea4a 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -366,7 +366,7 @@ const DatabaseModal: FunctionComponent = ({ ), ] - : null; + : []; useEffect(() => { if (show) { @@ -438,6 +438,7 @@ const DatabaseModal: FunctionComponent = ({ title={

{isEditMode ? t('Edit database') : t('Connect a database')}

} + footer={renderModalFooter()} > {isEditMode && ( From 145b947ab6082307550830dba7ac75fe6db01bbc Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 1 Jun 2021 15:18:17 -0700 Subject: [PATCH 042/145] # This is a combination of 6 commits. # This is the 1st commit message: feat: validation db modal (#14832) * split db modal file * hook up available databases * use new validation component # This is the commit message #2: feat: Icon Button (#14818) * Creating IconButton * Changed naming: logo is now icon * Hard-coded inset values for ellipses * Removed default SVG * Fixed test * Removed logo from test # This is the commit message #3: chore: Improves the native filters UI/UX - iteration 6 (#14932) # This is the commit message #4: fix: is_temporal should overwrite is_dttm (#14894) * fix: is_temporal should overwrite is_dttm * move up # This is the commit message #5: fix: time parser truncate to first day of year/month (#14945) # This is the commit message #6: hook up available databases --- .../database/DatabaseModal/index.test.jsx | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index 1fc51e45a584..86967257dd26 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -319,5 +319,72 @@ describe('DatabaseModal', () => { const todoText = screen.getAllByText(/todo/i); expect(todoText[0]).toBeVisible(); }); + + describe('create database', () => { + it('should show a form when dynamic_form is selected', async () => { + const props = { + ...dbProps, + databaseId: null, + database_name: null, + sqlalchemy_uri: null, + }; + render(, { useRedux: true }); + // it should have the correct header text + const headerText = screen.getByText(/connect a database/i); + expect(headerText).toBeVisible(); + + await screen.findByText(/display name/i); + + // it does not fetch any databases if no id is passed in + expect(fetchMock.calls().length).toEqual(0); + + // todo we haven't hooked this up to load dynamically yet so + // we can't currently test it + }); + }); + + describe('edit database', () => { + it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { + render(, { useRedux: true }); + + // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + expect(tabs[0]).toHaveTextContent('Basic'); + expect(tabs[1]).toHaveTextContent('Advanced'); + + // it should have the correct header text + const headerText = screen.getByText(/edit database/i); + expect(headerText).toBeVisible(); + + // todo add more when this form is built out + }); + it('renders the dynamic form when the dynamic_form configuration method is set', async () => { + fetchMock.get(DATABASE_FETCH_ENDPOINT, { + result: { + id: 10, + database_name: 'my database', + expose_in_sqllab: false, + allow_ctas: false, + allow_cvas: false, + configuration_method: 'dynamic_form', + parameters: { + database: 'mydatabase', + }, + }, + }); + render(, { useRedux: true }); + + await screen.findByText(/todo/i); + + // // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + + // it should show a TODO for now + const todoText = screen.getAllByText(/todo/i); + expect(todoText[0]).toBeVisible(); + }); + }); }); }); From cea7b6cc2fa2d69f0f50392dae1b2e90d738d946 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Wed, 2 Jun 2021 12:34:41 -0400 Subject: [PATCH 043/145] fix test for db modal --- .../database/DatabaseModal/index.test.jsx | 67 ------------------- 1 file changed, 67 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index 86967257dd26..1fc51e45a584 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -319,72 +319,5 @@ describe('DatabaseModal', () => { const todoText = screen.getAllByText(/todo/i); expect(todoText[0]).toBeVisible(); }); - - describe('create database', () => { - it('should show a form when dynamic_form is selected', async () => { - const props = { - ...dbProps, - databaseId: null, - database_name: null, - sqlalchemy_uri: null, - }; - render(, { useRedux: true }); - // it should have the correct header text - const headerText = screen.getByText(/connect a database/i); - expect(headerText).toBeVisible(); - - await screen.findByText(/display name/i); - - // it does not fetch any databases if no id is passed in - expect(fetchMock.calls().length).toEqual(0); - - // todo we haven't hooked this up to load dynamically yet so - // we can't currently test it - }); - }); - - describe('edit database', () => { - it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { - render(, { useRedux: true }); - - // it should have tabs - const tabs = screen.getAllByRole('tab'); - expect(tabs.length).toEqual(2); - expect(tabs[0]).toHaveTextContent('Basic'); - expect(tabs[1]).toHaveTextContent('Advanced'); - - // it should have the correct header text - const headerText = screen.getByText(/edit database/i); - expect(headerText).toBeVisible(); - - // todo add more when this form is built out - }); - it('renders the dynamic form when the dynamic_form configuration method is set', async () => { - fetchMock.get(DATABASE_FETCH_ENDPOINT, { - result: { - id: 10, - database_name: 'my database', - expose_in_sqllab: false, - allow_ctas: false, - allow_cvas: false, - configuration_method: 'dynamic_form', - parameters: { - database: 'mydatabase', - }, - }, - }); - render(, { useRedux: true }); - - await screen.findByText(/todo/i); - - // // it should have tabs - const tabs = screen.getAllByRole('tab'); - expect(tabs.length).toEqual(2); - - // it should show a TODO for now - const todoText = screen.getAllByText(/todo/i); - expect(todoText[0]).toBeVisible(); - }); - }); }); }); From d5c5167116416b66a1919e5be00d46c01890c037 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Tue, 1 Jun 2021 14:00:46 -0400 Subject: [PATCH 044/145] feat(db-connection-ui): Allow users to pick engine (#14884) * poc picker for db selection * working select * setup is loading for available dbs and step1 view * fix on close * update on fetch * remove unneeded code * add some styls --- .../data/database/DatabaseModal/index.tsx | 49 +++++++++++++++---- .../data/database/DatabaseModal/styles.ts | 4 ++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 265a8561923c..e73ea12908d6 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -25,7 +25,7 @@ import React, { Reducer, } from 'react'; import Tabs from 'src/components/Tabs'; -import { Alert } from 'src/common/components'; +import { Alert, Select } from 'src/common/components'; import Modal from 'src/components/Modal'; import Button from 'src/components/Button'; import withToasts from 'src/messageToasts/enhancers/withToasts'; @@ -59,6 +59,7 @@ import { formHelperStyles, formStyles, StyledBasicTab, + SelectDatabaseStyles, } from './styles'; const DOCUMENTATION_LINK = @@ -119,7 +120,7 @@ type DBReducerActionType = | { type: ActionType.configMethodChange; payload: { configuration_method: CONFIGURATION_METHOD }; - }; + } function dbReducer( state: Partial | null, @@ -166,13 +167,16 @@ function dbReducer( ...action.payload, }; case ActionType.dbSelected: + return { + ...action.payload, + }; case ActionType.configMethodChange: return { ...action.payload, }; case ActionType.reset: default: - return {}; + return null; } } @@ -195,6 +199,7 @@ const DatabaseModal: FunctionComponent = ({ const [validationErrors, getValidation] = useDatabaseValidation(); const [hasConnectedDb, setHasConnectedDb] = useState(false); const [dbName, setDbName] = useState(''); + const [isLoading, setLoading] = useState(false); const conf = useCommonConf(); const isEditMode = !!databaseId; @@ -298,12 +303,7 @@ const DatabaseModal: FunctionComponent = ({ if (show) { setTabKey(DEFAULT_TAB_KEY); getAvailableDbs(); - setDB({ - type: ActionType.dbSelected, - payload: { - configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI, - }, // todo hook this up to step 1 - }); + setLoading(true); } if (databaseId && show) { fetchDB(); @@ -322,6 +322,12 @@ const DatabaseModal: FunctionComponent = ({ } }, [dbFetched]); + useEffect(() => { + if (isLoading) { + setLoading(false); + } + }, [availableDbs, isLoading]); + const tabChange = (key: string) => { setTabKey(key); }; @@ -518,6 +524,31 @@ const DatabaseModal: FunctionComponent = ({ getValidation={() => getValidation(db)} validationErrors={validationErrors} /> + {!isLoading && !db && ( + + + + + )} {/* Step 2 */} -======= - {availableDbs?.databases?.map((database: DatabaseForm) => ( - - {database.name} - - ))} - - ->>>>>>> b8e668712e64993e700e83f7b3b76a44bf515969 )} )} From c0a8c74c5292d352f603aa31b9e13654cb7b83a0 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 1 Jun 2021 15:18:17 -0700 Subject: [PATCH 051/145] # This is a combination of 6 commits. # This is the 1st commit message: feat: validation db modal (#14832) * split db modal file * hook up available databases * use new validation component # This is the commit message #2: feat: Icon Button (#14818) * Creating IconButton * Changed naming: logo is now icon * Hard-coded inset values for ellipses * Removed default SVG * Fixed test * Removed logo from test # This is the commit message #3: chore: Improves the native filters UI/UX - iteration 6 (#14932) # This is the commit message #4: fix: is_temporal should overwrite is_dttm (#14894) * fix: is_temporal should overwrite is_dttm * move up # This is the commit message #5: fix: time parser truncate to first day of year/month (#14945) # This is the commit message #6: hook up available databases --- .../database/DatabaseModal/index.test.jsx | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index 1fc51e45a584..86967257dd26 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -319,5 +319,72 @@ describe('DatabaseModal', () => { const todoText = screen.getAllByText(/todo/i); expect(todoText[0]).toBeVisible(); }); + + describe('create database', () => { + it('should show a form when dynamic_form is selected', async () => { + const props = { + ...dbProps, + databaseId: null, + database_name: null, + sqlalchemy_uri: null, + }; + render(, { useRedux: true }); + // it should have the correct header text + const headerText = screen.getByText(/connect a database/i); + expect(headerText).toBeVisible(); + + await screen.findByText(/display name/i); + + // it does not fetch any databases if no id is passed in + expect(fetchMock.calls().length).toEqual(0); + + // todo we haven't hooked this up to load dynamically yet so + // we can't currently test it + }); + }); + + describe('edit database', () => { + it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { + render(, { useRedux: true }); + + // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + expect(tabs[0]).toHaveTextContent('Basic'); + expect(tabs[1]).toHaveTextContent('Advanced'); + + // it should have the correct header text + const headerText = screen.getByText(/edit database/i); + expect(headerText).toBeVisible(); + + // todo add more when this form is built out + }); + it('renders the dynamic form when the dynamic_form configuration method is set', async () => { + fetchMock.get(DATABASE_FETCH_ENDPOINT, { + result: { + id: 10, + database_name: 'my database', + expose_in_sqllab: false, + allow_ctas: false, + allow_cvas: false, + configuration_method: 'dynamic_form', + parameters: { + database: 'mydatabase', + }, + }, + }); + render(, { useRedux: true }); + + await screen.findByText(/todo/i); + + // // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + + // it should show a TODO for now + const todoText = screen.getAllByText(/todo/i); + expect(todoText[0]).toBeVisible(); + }); + }); }); }); From 907165f13fdcb87bff4cc621175dc434ff136d41 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Wed, 2 Jun 2021 12:34:41 -0400 Subject: [PATCH 052/145] fix test for db modal --- .../database/DatabaseModal/index.test.jsx | 67 ------------------- 1 file changed, 67 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index 86967257dd26..1fc51e45a584 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx @@ -319,72 +319,5 @@ describe('DatabaseModal', () => { const todoText = screen.getAllByText(/todo/i); expect(todoText[0]).toBeVisible(); }); - - describe('create database', () => { - it('should show a form when dynamic_form is selected', async () => { - const props = { - ...dbProps, - databaseId: null, - database_name: null, - sqlalchemy_uri: null, - }; - render(, { useRedux: true }); - // it should have the correct header text - const headerText = screen.getByText(/connect a database/i); - expect(headerText).toBeVisible(); - - await screen.findByText(/display name/i); - - // it does not fetch any databases if no id is passed in - expect(fetchMock.calls().length).toEqual(0); - - // todo we haven't hooked this up to load dynamically yet so - // we can't currently test it - }); - }); - - describe('edit database', () => { - it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { - render(, { useRedux: true }); - - // it should have tabs - const tabs = screen.getAllByRole('tab'); - expect(tabs.length).toEqual(2); - expect(tabs[0]).toHaveTextContent('Basic'); - expect(tabs[1]).toHaveTextContent('Advanced'); - - // it should have the correct header text - const headerText = screen.getByText(/edit database/i); - expect(headerText).toBeVisible(); - - // todo add more when this form is built out - }); - it('renders the dynamic form when the dynamic_form configuration method is set', async () => { - fetchMock.get(DATABASE_FETCH_ENDPOINT, { - result: { - id: 10, - database_name: 'my database', - expose_in_sqllab: false, - allow_ctas: false, - allow_cvas: false, - configuration_method: 'dynamic_form', - parameters: { - database: 'mydatabase', - }, - }, - }); - render(, { useRedux: true }); - - await screen.findByText(/todo/i); - - // // it should have tabs - const tabs = screen.getAllByRole('tab'); - expect(tabs.length).toEqual(2); - - // it should show a TODO for now - const todoText = screen.getAllByText(/todo/i); - expect(todoText[0]).toBeVisible(); - }); - }); }); }); From e3a87b9860e5333d6b560d38344c3556718ac773 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Tue, 1 Jun 2021 14:00:46 -0400 Subject: [PATCH 053/145] feat(db-connection-ui): Allow users to pick engine (#14884) * poc picker for db selection * working select * setup is loading for available dbs and step1 view * fix on close * update on fetch * remove unneeded code * add some styls --- .../data/database/DatabaseModal/index.tsx | 49 +++++++++++++++---- .../data/database/DatabaseModal/styles.ts | 4 ++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 265a8561923c..e73ea12908d6 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -25,7 +25,7 @@ import React, { Reducer, } from 'react'; import Tabs from 'src/components/Tabs'; -import { Alert } from 'src/common/components'; +import { Alert, Select } from 'src/common/components'; import Modal from 'src/components/Modal'; import Button from 'src/components/Button'; import withToasts from 'src/messageToasts/enhancers/withToasts'; @@ -59,6 +59,7 @@ import { formHelperStyles, formStyles, StyledBasicTab, + SelectDatabaseStyles, } from './styles'; const DOCUMENTATION_LINK = @@ -119,7 +120,7 @@ type DBReducerActionType = | { type: ActionType.configMethodChange; payload: { configuration_method: CONFIGURATION_METHOD }; - }; + } function dbReducer( state: Partial | null, @@ -166,13 +167,16 @@ function dbReducer( ...action.payload, }; case ActionType.dbSelected: + return { + ...action.payload, + }; case ActionType.configMethodChange: return { ...action.payload, }; case ActionType.reset: default: - return {}; + return null; } } @@ -195,6 +199,7 @@ const DatabaseModal: FunctionComponent = ({ const [validationErrors, getValidation] = useDatabaseValidation(); const [hasConnectedDb, setHasConnectedDb] = useState(false); const [dbName, setDbName] = useState(''); + const [isLoading, setLoading] = useState(false); const conf = useCommonConf(); const isEditMode = !!databaseId; @@ -298,12 +303,7 @@ const DatabaseModal: FunctionComponent = ({ if (show) { setTabKey(DEFAULT_TAB_KEY); getAvailableDbs(); - setDB({ - type: ActionType.dbSelected, - payload: { - configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI, - }, // todo hook this up to step 1 - }); + setLoading(true); } if (databaseId && show) { fetchDB(); @@ -322,6 +322,12 @@ const DatabaseModal: FunctionComponent = ({ } }, [dbFetched]); + useEffect(() => { + if (isLoading) { + setLoading(false); + } + }, [availableDbs, isLoading]); + const tabChange = (key: string) => { setTabKey(key); }; @@ -518,6 +524,31 @@ const DatabaseModal: FunctionComponent = ({ getValidation={() => getValidation(db)} validationErrors={validationErrors} /> + {!isLoading && !db && ( + + + + + )}