You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2021/08/10 17:08:22 UTC
[superset] branch master updated: fix: Make sheet_name into a
`ValidationInputError` (#16056)
This is an automated email from the ASF dual-hosted git repository.
hugh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new fd80ae3 fix: Make sheet_name into a `ValidationInputError` (#16056)
fd80ae3 is described below
commit fd80ae34a35f06160240a0dbf47cd6190053f6e7
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Tue Aug 10 13:07:31 2021 -0400
fix: Make sheet_name into a `ValidationInputError` (#16056)
* setup validates for name
* add error type
* fix linting
* fix test
* remove errors
* fix number
* fix test
---
.../DatabaseModal/DatabaseConnectionForm.tsx | 12 ++++---
.../CRUD/data/database/DatabaseModal/styles.ts | 5 +--
superset-frontend/src/views/CRUD/hooks.ts | 39 ++++++++++++++++++----
superset/db_engine_specs/gsheets.py | 27 +++++++++------
tests/unit_tests/db_engine_specs/test_gsheets.py | 31 +++--------------
5 files changed, 64 insertions(+), 50 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..82cc466 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')}
</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 => {
+ onChange={(e: { target: { value: any } }) => {
changeMethods.onParametersChange({
target: {
type: `catalog-${idx}`,
@@ -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..808d124 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.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,
+ error_type,
+ [extra.catalog.idx]: {
+ name: message,
+ },
+ };
+ }
+ if (extra.catalog.url) {
return {
...obj,
- [extra.name]: message,
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) {
return {
...obj,
[extra.invalid[0]]: message,
diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py
index 774924f..747262c 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 bd11375..0caa7af 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,