This is an automated email from the ASF dual-hosted git repository. hugh pushed a commit to branch hugh/gsheets-name-validation in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3e6b97b6dbb6c970e3e78b1ed6ed58ecf6660268 Author: hughhhh <[email protected]> AuthorDate: Wed Aug 4 00:32:41 2021 -0400 setup validates for name --- .../DatabaseModal/DatabaseConnectionForm.tsx | 10 +++--- .../CRUD/data/database/DatabaseModal/styles.ts | 5 +-- superset-frontend/src/views/CRUD/hooks.ts | 38 ++++++++++++++++++---- superset/db_engine_specs/gsheets.py | 22 ++++++++++--- 4 files changed, 58 insertions(+), 17 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 f6997fc..4dc85b8 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,8 +222,11 @@ const TableCatalog = ({ {t('Google Sheet Name and URL')} </FormLabel> <div className="catalog-name"> - <Input + <ValidatedInput className="catalog-name-input" + required={required} + validationMethods={{ onBlur: getValidation }} + errorMessage={catalogError[idx]?.name} placeholder={t('Enter a name for this sheet')} onChange={e => { changeMethods.onParametersChange({ @@ -236,7 +239,6 @@ const TableCatalog = ({ }} value={sheet.name} /> - {tableCatalog?.length > 1 && ( <CloseOutlined className="catalog-delete" @@ -248,7 +250,7 @@ const TableCatalog = ({ className="catalog-name-url" required={required} validationMethods={{ onBlur: getValidation }} - errorMessage={catalogError[sheet.name]} + errorMessage={catalogError[idx]?.url} placeholder={t('Paste the shareable Google Sheet URL here')} onChange={(e: { target: { value: any } }) => 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 9a4a2a1..364ee97 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 c1ee1f5..db1af72 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -678,21 +678,45 @@ export function useDatabaseValidation() { invalid?: string[]; missing?: string[]; name: string; + catalog: { + name: string; + url: string; + idx: string; + }; }; message: string; }, ) => { - // 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') { + if (extra.catalog) { + if (extra.catalog.name) { + return { + ...obj, + [extra.catalog.idx]: { + name: message, + }, + }; + } + if (extra.catalog.url) { return { ...obj, - [extra.name]: message, - error_type, + [extra.catalog.idx]: { + url: message, + }, }; } + + return { + ...obj, + [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) { return { ...obj, [extra.invalid[0]]: message, diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index 774924f..2abe193 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -153,10 +153,10 @@ class GSheetsEngineSpec(SqliteEngineSpec): if not table_catalog: errors.append( SupersetError( - message="URL is required", + message="Missing required field", error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, level=ErrorLevel.WARNING, - extra={"invalid": ["catalog"], "name": "", "url": ""}, + extra={"catalog": {"idx": 0}}, ), ) return errors @@ -171,6 +171,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 +180,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 +205,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
