diff --git a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx index 272cbc23b..9666dcfdb 100644 --- a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx +++ b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx @@ -312,6 +312,14 @@ The results stored in the backend were stored in a different format, and no long The query results were stored in a format that is no longer supported. Please re-run your query. +## Issue 1034 + +``` +The database port provided is invalid. +``` + +Please check that the provided database port is an integer between 0 and 65535 (inclusive). + ## Issue 1035 ``` 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 e29ec48a9..225502167 100644 --- a/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts @@ -21,71 +21,50 @@ import { DATABASE_LIST } from './helper'; describe('Add database', () => { beforeEach(() => { cy.login(); - }); - - it('should keep create modal open when error', () => { cy.visit(DATABASE_LIST); - - // open modal + cy.wait(3000); 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', - '', - ); - - // 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(); - - // 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'); }); - it('should keep update modal open when error', () => { - // open modal - cy.get('[data-test="database-edit"]:last').click(); + it('should open dynamic form', () => { + // click postgres dynamic form + cy.get('.preferred > :nth-child(1)').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'); + // make sure all the fields are rendering + cy.get('input[name="host"]').should('have.value', ''); + cy.get('input[name="port"]').should('have.value', ''); + cy.get('input[name="database"]').should('have.value', ''); + cy.get('input[name="password"]').should('have.value', ''); + cy.get('input[name="database_name"]').should('have.value', ''); + }); - cy.get('[data-test="database-modal"]:last input[name="sqlalchemy_uri"]') - .focus() - .dblclick() - .type('{selectall}{backspace}bad_uri'); + it('should open sqlalchemy form', () => { + // click postgres dynamic form + cy.get('.preferred > :nth-child(1)').click(); - // click save - cy.get('[data-test="modal-confirm-button"]:not(:disabled)').click(); + cy.get('[data-test="sqla-connect-btn"]').click(); - // should show error alerts - // TODO(hugh): Update this test - // cy.get('.toast').contains('error').should('be.visible'); + // check if the sqlalchemy form is showing up + cy.get('[data-test=database-name-input]').should('be.visible'); + cy.get('[data-test="sqlalchemy-uri-input"]').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'); + it('show error alerts on dynamic form for bad host', () => { + // click postgres dynamic form + cy.get('.preferred > :nth-child(1)').click(); + cy.get('input[name="host"]').focus().type('badhost'); + cy.get('input[name="port"]').focus().type('5432'); + cy.get('.ant-form-item-explain-error').contains( + "The hostname provided can't be resolved", + ); + }); + + it('show error alerts on dynamic form for bad port', () => { + // click postgres dynamic form + cy.get('.preferred > :nth-child(1)').click(); + cy.get('input[name="host"]').focus().type('localhost'); + cy.get('input[name="port"]').focus().type('123'); + cy.get('input[name="database"]').focus(); + cy.get('.ant-form-item-explain-error').contains('The port is closed'); }); }); diff --git a/superset-frontend/images/icons/default_db_image.svg b/superset-frontend/images/icons/default_db_image.svg new file mode 100644 index 000000000..f8892bb31 --- /dev/null +++ b/superset-frontend/images/icons/default_db_image.svg @@ -0,0 +1,21 @@ + + + + diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 98aa7d12a..b73fa9b69 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -99,6 +99,7 @@ "react-js-cron": "^1.2.0", "react-json-tree": "^0.11.2", "react-jsonschema-form": "^1.2.0", + "react-lines-ellipsis": "^0.15.0", "react-loadable": "^5.5.0", "react-markdown": "^4.3.1", "react-redux": "^7.2.0", @@ -44276,6 +44277,15 @@ "resolved": "https://registry.npmjs.org/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz", "integrity": "sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==" }, + "node_modules/react-lines-ellipsis": { + "version": "0.15.0", + "resolved": "https://registry.npmjs.org/react-lines-ellipsis/-/react-lines-ellipsis-0.15.0.tgz", + "integrity": "sha512-8kWpEmu7ijmB6Gz5t+eSjNux2SpVXZBsmfeFE8LjMS7tU3H8ai475CyNc0dH0RDTwt4Esr7c06Xq4SB7Gpl9yQ==", + "peerDependencies": { + "react": "*", + "react-dom": "*" + } + }, "node_modules/react-loadable": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/react-loadable/-/react-loadable-5.5.0.tgz", @@ -89083,6 +89093,12 @@ "resolved": "https://registry.npmjs.org/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz", "integrity": "sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==" }, + "react-lines-ellipsis": { + "version": "0.15.0", + "resolved": "https://registry.npmjs.org/react-lines-ellipsis/-/react-lines-ellipsis-0.15.0.tgz", + "integrity": "sha512-8kWpEmu7ijmB6Gz5t+eSjNux2SpVXZBsmfeFE8LjMS7tU3H8ai475CyNc0dH0RDTwt4Esr7c06Xq4SB7Gpl9yQ==", + "requires": {} + }, "react-loadable": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/react-loadable/-/react-loadable-5.5.0.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 846e0cdea..1d4aa8944 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -151,6 +151,7 @@ "react-js-cron": "^1.2.0", "react-json-tree": "^0.11.2", "react-jsonschema-form": "^1.2.0", + "react-lines-ellipsis": "^0.15.0", "react-loadable": "^5.5.0", "react-markdown": "^4.3.1", "react-redux": "^7.2.0", diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index a5699b20f..496a9b4ce 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -33,6 +33,7 @@ export const ErrorTypeEnum = { CONNECTION_INVALID_PASSWORD_ERROR: 'CONNECTION_INVALID_PASSWORD_ERROR', CONNECTION_INVALID_HOSTNAME_ERROR: 'CONNECTION_INVALID_HOSTNAME_ERROR', CONNECTION_PORT_CLOSED_ERROR: 'CONNECTION_PORT_CLOSED_ERROR', + CONNECTION_INVALID_PORT_ERROR: 'CONNECTION_INVALID_PORT_ERROR', CONNECTION_HOST_DOWN_ERROR: 'CONNECTION_HOST_DOWN_ERROR', CONNECTION_ACCESS_DENIED_ERROR: 'CONNECTION_ACCESS_DENIED_ERROR', CONNECTION_UNKNOWN_DATABASE_ERROR: 'CONNECTION_UNKNOWN_DATABASE_ERROR', diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.stories.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.stories.tsx index 6061848d8..d1c81b9af 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.stories.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.stories.tsx @@ -32,6 +32,7 @@ export const InteractiveLabeledErrorBoundInput = ({ placeholder, type, id, + tooltipText, }: LabeledErrorBoundInputProps) => { const [currentValue, setCurrentValue] = useState(value); @@ -58,6 +59,8 @@ export const InteractiveLabeledErrorBoundInput = ({ placeholder={placeholder} type={type} required + hasTooltip + tooltipText={tooltipText} /> ); }; @@ -66,6 +69,7 @@ InteractiveLabeledErrorBoundInput.args = { name: 'Username', placeholder: 'Example placeholder text...', id: 1, + tooltipText: 'This is a tooltip', }; InteractiveLabeledErrorBoundInput.argTypes = { diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.test.jsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.test.jsx index 15f956fd5..ebfd2b30e 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.test.jsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.test.jsx @@ -17,7 +17,7 @@ * under the License. */ import React from 'react'; -import { render, screen } from 'spec/helpers/testing-library'; +import { render, fireEvent, screen } from 'spec/helpers/testing-library'; import LabeledErrorBoundInput from 'src/components/Form/LabeledErrorBoundInput'; const defaultProps = { @@ -27,6 +27,8 @@ const defaultProps = { validationMethods: () => {}, errorMessage: '', helpText: 'This is a line of example help text', + hasTooltip: false, + tooltipText: 'This is a tooltip', value: '', placeholder: 'Example placeholder text...', type: 'textbox', @@ -58,4 +60,19 @@ describe('LabeledErrorBoundInput', () => { expect(textboxInput).toBeVisible(); expect(errorText).toBeVisible(); }); + it('renders a LabledErrorBoundInput with a InfoTooltip', async () => { + defaultProps.hasTooltip = true; + render(); + + const label = screen.getByText(/username/i); + const textboxInput = screen.getByRole('textbox'); + const tooltipIcon = screen.getByRole('img'); + + fireEvent.mouseOver(tooltipIcon); + + expect(tooltipIcon).toBeVisible(); + expect(label).toBeVisible(); + expect(textboxInput).toBeVisible(); + expect(await screen.findByText('This is a tooltip')).toBeInTheDocument(); + }); }); diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx index 75df2bb08..f654c4f5d 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx @@ -19,6 +19,8 @@ import React from 'react'; import { Input } from 'antd'; import { styled, css, SupersetTheme } from '@superset-ui/core'; +import InfoTooltip from 'src/components/InfoTooltip'; +import errorIcon from 'images/icons/error.svg'; import FormItem from './FormItem'; import FormLabel from './FormLabel'; @@ -30,6 +32,8 @@ export interface LabeledErrorBoundInputProps { errorMessage: string | null; helpText?: string; required?: boolean; + hasTooltip?: boolean; + tooltipText?: string | null; id?: string; classname?: string; [x: string]: any; @@ -46,42 +50,63 @@ const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css` ${hasError && `.ant-form-item-control-input-content { position: relative; - &:after { content: ' '; display: inline-block; background: ${theme.colors.error.base}; - mask: url('/images/icons/error.svg'); + mask: url(${errorIcon}); mask-size: cover; width: ${theme.gridUnit * 4}px; height: ${theme.gridUnit * 4}px; position: absolute; - right: 7px; - top: 15px; + right: ${theme.gridUnit * 1.25}px; + top: ${theme.gridUnit * 2.75}px; } }`} `; + const StyledFormGroup = styled('div')` + input::-webkit-outer-spin-button, + input::-webkit-inner-spin-button { + -webkit-appearance: none; + margin: 0; + } margin-bottom: ${({ theme }) => theme.gridUnit * 5}px; .ant-form-item { margin-bottom: 0; } `; +const infoTooltip = (theme: SupersetTheme) => css` + svg { + vertical-align: bottom; + margin-bottom: ${theme.gridUnit * 0.25}px; + } +`; + const LabeledErrorBoundInput = ({ label, validationMethods, errorMessage, helpText, required = false, + hasTooltip = false, + tooltipText, id, className, ...props }: LabeledErrorBoundInputProps) => ( - + infoTooltip(theme)} + > {label} + {hasTooltip && ( + + )} alertIconStyles(theme, !!errorMessage)} validateTrigger={Object.keys(validationMethods)} diff --git a/superset-frontend/src/components/Icon/index.tsx b/superset-frontend/src/components/Icon/index.tsx index 693354d9a..e83dde113 100644 --- a/superset-frontend/src/components/Icon/index.tsx +++ b/superset-frontend/src/components/Icon/index.tsx @@ -145,6 +145,7 @@ import { ReactComponent as WarningIcon } from 'images/icons/warning.svg'; import { ReactComponent as WarningSolidIcon } from 'images/icons/warning_solid.svg'; import { ReactComponent as XLargeIcon } from 'images/icons/x-large.svg'; import { ReactComponent as XSmallIcon } from 'images/icons/x-small.svg'; +import { ReactComponent as DefaultDatabaseIcon } from 'images/icons/default_db_image.svg'; export type IconName = | 'alert' @@ -184,6 +185,7 @@ export type IconName = | 'copy' | 'cursor-target' | 'database' + | 'default-database' | 'dataset-physical' | 'dataset-virtual' | 'dataset-virtual-greyscale' @@ -299,6 +301,7 @@ export const iconsRegistry: Record< 'circle-check-solid': CircleCheckSolidIcon, 'color-palette': ColorPaletteIcon, 'cursor-target': CursorTargeIcon, + 'default-database': DefaultDatabaseIcon, 'dataset-physical': DatasetPhysicalIcon, 'dataset-virtual': DatasetVirtualIcon, 'dataset-virtual-greyscale': DatasetVirtualGreyscaleIcon, diff --git a/superset-frontend/src/components/IconButton/index.tsx b/superset-frontend/src/components/IconButton/index.tsx index e7f9c2d89..d36015e29 100644 --- a/superset-frontend/src/components/IconButton/index.tsx +++ b/superset-frontend/src/components/IconButton/index.tsx @@ -17,9 +17,11 @@ * under the License. */ import React from 'react'; -import { styled } from '@superset-ui/core'; +import { styled, supersetTheme } from '@superset-ui/core'; import Button from 'src/components/Button'; import { ButtonProps as AntdButtonProps } from 'antd/lib/button'; +import Icon from 'src/components/Icon'; +import LinesEllipsis from 'react-lines-ellipsis'; export interface IconButtonProps extends AntdButtonProps { buttonText: string; @@ -33,17 +35,25 @@ const StyledButton = styled(Button)` flex-direction: column; padding: 0; `; + const StyledImage = styled.div` - margin: ${({ theme }) => theme.gridUnit * 8}px 0; padding: ${({ theme }) => theme.gridUnit * 4}px; + height: ${({ theme }) => theme.gridUnit * 18}px; + margin: ${({ theme }) => theme.gridUnit * 3}px 0; &:first-of-type { margin-right: 0; } img { - width: fit-content; - + width: ${({ theme }) => theme.gridUnit * 10}px; + height: ${({ theme }) => theme.gridUnit * 10}px; + margin: 0; + &:first-of-type { + margin-right: 0; + } + } + svg { &:first-of-type { margin-right: 0; } @@ -52,32 +62,21 @@ const StyledImage = styled.div` const StyledInner = styled.div` max-height: calc(1.5em * 2); - overflow: hidden; - padding-right: 1rem; - position: relative; white-space: break-spaces; - &::before { - content: '...'; - inset-block-end: 0; /* "bottom" */ - inset-inline-end: 8px; /* "right" */ - position: absolute; + &:first-of-type { + margin-right: 0; } - &::after { - background-color: ${({ theme }) => theme.colors.grayscale.light4}; - content: ''; - height: 1rem; - inset-inline-end: 8px; /* "right" */ - position: absolute; - top: 4px; - width: 1rem; + .LinesEllipsis { + &:first-of-type { + margin-right: 0; + } } `; const StyledBottom = styled.div` - padding: ${({ theme }) => theme.gridUnit * 6}px - ${({ theme }) => theme.gridUnit * 4}px; + padding: ${({ theme }) => theme.gridUnit * 4}px 0; border-radius: 0 0 ${({ theme }) => theme.borderRadius}px ${({ theme }) => theme.borderRadius}px; background-color: ${({ theme }) => theme.colors.grayscale.light4}; @@ -96,10 +95,27 @@ const IconButton = styled( ({ icon, altText, buttonText, ...props }: IconButtonProps) => ( - {altText} + {icon && {altText}} + {!icon && ( + + )} + - {buttonText} + + + ), @@ -117,6 +133,7 @@ const IconButton = styled( background-color: ${({ theme }) => theme.colors.grayscale.light5}; color: ${({ theme }) => theme.colors.grayscale.dark2}; border: 1px solid ${({ theme }) => theme.colors.grayscale.light2}; + box-shadow: 4px 4px 20px ${({ theme }) => theme.colors.grayscale.light2}; } `; diff --git a/superset-frontend/src/components/InfoTooltip/index.tsx b/superset-frontend/src/components/InfoTooltip/index.tsx index 369d2e931..7aa9afaeb 100644 --- a/superset-frontend/src/components/InfoTooltip/index.tsx +++ b/superset-frontend/src/components/InfoTooltip/index.tsx @@ -42,11 +42,11 @@ export interface InfoTooltipProps { trigger?: string | Array; overlayStyle?: any; bgColor?: string; + viewBox?: string; } const StyledTooltip = styled(Tooltip)` cursor: pointer; - path:first-of-type { fill: #999999; } @@ -74,7 +74,7 @@ export default function InfoTooltip({ overlayStyle={overlayStyle} color={bgColor} > - + ); } diff --git a/superset-frontend/src/types/react-lines-ellipsis.d.ts b/superset-frontend/src/types/react-lines-ellipsis.d.ts new file mode 100644 index 000000000..a0772bacc --- /dev/null +++ b/superset-frontend/src/types/react-lines-ellipsis.d.ts @@ -0,0 +1,47 @@ +/** + * 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. + */ + +declare module 'react-lines-ellipsis' { + interface ReactLinesEllipsisProps { + basedOn?: 'letters' | 'words'; + className?: string; + component?: string; + ellipsis?: string; + isClamped?: () => boolean; + maxLine?: number | string; + onReflow?: ({ clamped, text }: { clamped: boolean; text: string }) => any; + style?: React.CSSProperties; + text?: string; + trimRight?: boolean; + winWidth?: number; + } + + // eslint-disable-next-line react/prefer-stateless-function + class LinesEllipsis extends React.Component { + static defaultProps?: ReactLinesEllipsisProps; + } + + export default LinesEllipsis; +} + +declare module 'react-lines-ellipsis/lib/responsiveHOC' { + export default function responsiveHOC():

( + WrappedComponent: React.ComponentType

, + ) => React.ComponentClass

; +} 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 5c7c729da..9125d4a02 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx @@ -16,16 +16,27 @@ * specific language governing permissions and limitations * under the License. */ -import React, { FormEvent } from 'react'; -import { SupersetTheme, JsonObject } from '@superset-ui/core'; +import React, { FormEvent, useState } from 'react'; +import { SupersetTheme, JsonObject, t } from '@superset-ui/core'; import { InputProps } from 'antd/lib/input'; +import { Switch, Select, Button } from 'src/common/components'; +import InfoTooltip from 'src/components/InfoTooltip'; import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput'; +import FormLabel from 'src/components/Form/FormLabel'; +import { DeleteFilled } from '@ant-design/icons'; import { - StyledFormHeader, formScrollableStyles, validatedFormStyles, + CredentialInfoForm, + toggleStyle, + infoTooltip, } from './styles'; -import { DatabaseForm } from '../types'; +import { DatabaseForm, DatabaseObject } from '../types'; + +enum CredentialInfoOptions { + jsonUpload, + copyPaste, +} export const FormFieldOrder = [ 'host', @@ -34,27 +45,171 @@ export const FormFieldOrder = [ 'username', 'password', 'database_name', + 'credentials_info', + 'query', + 'encryption', ]; interface FieldPropTypes { required: boolean; + hasTooltip?: boolean; + tooltipText?: (valuse: any) => string; + onParametersChange: (value: any) => string; + onParametersUploadFileChange: (value: any) => string; changeMethods: { onParametersChange: (value: any) => string } & { onChange: (value: any) => string; - }; + } & { onParametersUploadFileChange: (value: any) => string }; validationErrors: JsonObject | null; getValidation: () => void; + db?: DatabaseObject; + isEditMode?: boolean; + sslForced?: boolean; + defaultDBName?: string; + editNewDb?: boolean; } +const CredentialsInfo = ({ + changeMethods, + isEditMode, + db, + editNewDb, +}: FieldPropTypes) => { + const [uploadOption, setUploadOption] = useState( + CredentialInfoOptions.jsonUpload.valueOf(), + ); + const [fileToUpload, setFileToUpload] = useState( + null, + ); + return ( + + {!isEditMode && ( + <> + + {`${t('How do you want to enter service account credentials?')}`} + + + + )} + {uploadOption === CredentialInfoOptions.copyPaste || + isEditMode || + editNewDb ? ( +

+ {`${t('Service Account')}`} +