mirror of https://github.com/apache/superset.git
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
This commit is contained in:
Hugh A. Miles II
GitHub
parent
f0e3b68cc2
commit
fd80ae34a3
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue