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 f6997fc84e9..82cc4667e03 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx @@ -19,7 +19,7 @@ import React, { FormEvent, useState } from 'react'; import { SupersetTheme, JsonObject, t } from '@superset-ui/core'; import { InputProps } from 'antd/lib/input'; -import { Input, Switch, Select, Button } from 'src/common/components'; +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'; @@ -222,10 +222,13 @@ const TableCatalog = ({ {t('Google Sheet Name and URL')}
- { + onChange={(e: { target: { value: any } }) => { changeMethods.onParametersChange({ target: { type: `catalog-${idx}`, @@ -236,7 +239,6 @@ const TableCatalog = ({ }} value={sheet.name} /> - {tableCatalog?.length > 1 && ( changeMethods.onParametersChange({ 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 9a4a2a1237b..364ee971e12 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts @@ -552,13 +552,14 @@ export const StyledCatalogTable = styled.div` } .catalog-label { - margin: 0 0 8px; + margin: 0 0 7px; } .catalog-name { display: flex; .catalog-name-input { width: 95%; + margin-bottom: 0px; } } @@ -570,7 +571,7 @@ export const StyledCatalogTable = styled.div` .catalog-delete { align-self: center; background: ${({ theme }) => theme.colors.grayscale.light4}; - margin: 5px; + margin: 5px 5px 8px 5px; } .catalog-add-btn { diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index c1ee1f52c69..808d124247f 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -678,21 +678,48 @@ export function useDatabaseValidation() { invalid?: string[]; missing?: string[]; name: string; + catalog: { + name: string; + url: string; + idx: number; + }; }; message: string; }, ) => { + if (extra.catalog) { + if (extra.catalog.name) { + return { + ...obj, + error_type, + [extra.catalog.idx]: { + name: message, + }, + }; + } + if (extra.catalog.url) { + return { + ...obj, + error_type, + [extra.catalog.idx]: { + url: message, + }, + }; + } + + return { + ...obj, + error_type, + [extra.catalog.idx]: { + name: message, + url: message, + }, + }; + } // if extra.invalid doesn't exist then the // error can't be mapped to a parameter // so leave it alone if (extra.invalid) { - if (extra.invalid[0] === 'catalog') { - return { - ...obj, - [extra.name]: message, - error_type, - }; - } return { ...obj, [extra.invalid[0]]: message, diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index 774924fde71..747262ca228 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -151,14 +151,7 @@ class GSheetsEngineSpec(SqliteEngineSpec): table_catalog = parameters.get("catalog", {}) if not table_catalog: - errors.append( - SupersetError( - message="URL is required", - error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, - level=ErrorLevel.WARNING, - extra={"invalid": ["catalog"], "name": "", "url": ""}, - ), - ) + # Allowing users to submit empty catalogs return errors # We need a subject in case domain wide delegation is set, otherwise the @@ -171,6 +164,7 @@ class GSheetsEngineSpec(SqliteEngineSpec): "gsheets://", service_account_info=credentials_info, subject=subject, ) conn = engine.connect() + idx = 0 for name, url in table_catalog.items(): if not name: @@ -179,9 +173,21 @@ class GSheetsEngineSpec(SqliteEngineSpec): message="Sheet name is required", error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, level=ErrorLevel.WARNING, - extra={"invalid": [], "name": name, "url": url}, + extra={"catalog": {"idx": idx, "name": True}}, ), ) + return errors + + if not url: + errors.append( + SupersetError( + message="URL is required", + error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, + level=ErrorLevel.WARNING, + extra={"catalog": {"idx": idx, "url": True}}, + ), + ) + return errors try: results = conn.execute(f'SELECT * FROM "{url}" LIMIT 1') @@ -192,7 +198,8 @@ class GSheetsEngineSpec(SqliteEngineSpec): message="URL could not be identified", error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, level=ErrorLevel.WARNING, - extra={"invalid": ["catalog"], "name": name, "url": url}, + extra={"catalog": {"idx": idx, "url": True}}, ), ) + idx += 1 return errors diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py index bd11375bb68..0caa7afc8e4 100644 --- a/tests/unit_tests/db_engine_specs/test_gsheets.py +++ b/tests/unit_tests/db_engine_specs/test_gsheets.py @@ -40,24 +40,7 @@ def test_validate_parameters_simple( "catalog": {}, } errors = GSheetsEngineSpec.validate_parameters(parameters) - assert errors == [ - SupersetError( - message="URL is required", - error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, - level=ErrorLevel.WARNING, - extra={ - "invalid": ["catalog"], - "name": "", - "url": "", - "issue_codes": [ - { - "code": 1018, - "message": "Issue 1018 - One or more parameters needed to configure a database are missing.", - } - ], - }, - ) - ] + assert errors == [] def test_validate_parameters_catalog( @@ -96,9 +79,7 @@ def test_validate_parameters_catalog( error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, level=ErrorLevel.WARNING, extra={ - "invalid": ["catalog"], - "name": "private_sheet", - "url": "https://docs.google.com/spreadsheets/d/1/edit", + "catalog": {"idx": 0, "url": True,}, "issue_codes": [ { "code": 1003, @@ -116,9 +97,7 @@ def test_validate_parameters_catalog( error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, level=ErrorLevel.WARNING, extra={ - "invalid": ["catalog"], - "name": "not_a_sheet", - "url": "https://www.google.com/", + "catalog": {"idx": 2, "url": True,}, "issue_codes": [ { "code": 1003, @@ -173,9 +152,7 @@ def test_validate_parameters_catalog_and_credentials( error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, level=ErrorLevel.WARNING, extra={ - "invalid": ["catalog"], - "name": "not_a_sheet", - "url": "https://www.google.com/", + "catalog": {"idx": 2, "url": True,}, "issue_codes": [ { "code": 1003,