diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index de5d750ebd067..a2ca444752a55 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -569,6 +569,18 @@ const DatabaseModal: FunctionComponent = ({ // Clone DB object const dbToUpdate = JSON.parse(JSON.stringify(db || {})); + if (dbToUpdate.catalog) { + // convert catalog to fit /validate_parameters endpoint + dbToUpdate.catalog = Object.assign( + {}, + ...dbToUpdate.catalog.map((x: { name: string; value: string }) => ({ + [x.name]: x.value, + })), + ); + } else { + dbToUpdate.catalog = {}; + } + if (dbToUpdate.configuration_method === CONFIGURATION_METHOD.DYNAMIC_FORM) { // Validate DB before saving const errors = await getValidation(dbToUpdate, true); @@ -734,7 +746,10 @@ const DatabaseModal: FunctionComponent = ({ }); } - setDB({ type: ActionType.addTableCatalogSheet }); + if (database_name === 'Google Sheets') { + // only create a catalog if the DB is Google Sheets + setDB({ type: ActionType.addTableCatalogSheet }); + } }; const renderAvailableSelector = () => ( diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index 201e35cbfc881..dafd1ba7fc719 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -315,6 +315,11 @@ class Meta: # pylint: disable=too-few-public-methods values=fields.Raw(allow_none=True), description="DB-specific parameters for configuration", ) + catalog = fields.Dict( + keys=fields.String(), + values=fields.Raw(allow_none=True), + description="Gsheets specific column for managing label to sheet urls", + ) database_name = fields.String( description=database_name_description, allow_none=True, diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index 78b42d2b3a999..fd1a2754d76bc 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -55,11 +55,12 @@ class GSheetsParametersSchema(Schema): class GSheetsParametersType(TypedDict): service_account_info: str - catalog: Dict[str, str] + catalog: Optional[Dict[str, str]] class GSheetsPropertiesType(TypedDict): parameters: GSheetsParametersType + catalog: Dict[str, str] class GSheetsEngineSpec(SqliteEngineSpec): @@ -215,7 +216,15 @@ def validate_parameters( properties: GSheetsPropertiesType, ) -> List[SupersetError]: errors: List[SupersetError] = [] + + # backwards compatible just incase people are send data + # via parameters for validation parameters = properties.get("parameters", {}) + if parameters and parameters.get("catalog"): + table_catalog = parameters.get("catalog", {}) + else: + table_catalog = properties.get("catalog", {}) + encrypted_credentials = parameters.get("service_account_info") or "{}" # On create the encrypted credentials are a string, @@ -223,8 +232,6 @@ def validate_parameters( if isinstance(encrypted_credentials, str): encrypted_credentials = json.loads(encrypted_credentials) - table_catalog = parameters.get("catalog", {}) - if not table_catalog: # Allowing users to submit empty catalogs errors.append( @@ -250,6 +257,7 @@ def validate_parameters( ) conn = engine.connect() idx = 0 + for name, url in table_catalog.items(): if not name: diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py index 487e1eff695f8..042e486642bd8 100644 --- a/tests/unit_tests/db_engine_specs/test_gsheets.py +++ b/tests/unit_tests/db_engine_specs/test_gsheets.py @@ -40,7 +40,32 @@ def test_validate_parameters_simple() -> None: "parameters": { "service_account_info": "", "catalog": {}, - } + }, + "catalog": {}, + } + errors = GSheetsEngineSpec.validate_parameters(properties) + assert errors == [ + SupersetError( + message="Sheet name is required", + error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, + level=ErrorLevel.WARNING, + extra={"catalog": {"idx": 0, "name": True}}, + ), + ] + + +def test_validate_parameters_simple_with_in_root_catalog() -> None: + from superset.db_engine_specs.gsheets import ( + GSheetsEngineSpec, + GSheetsPropertiesType, + ) + + properties: GSheetsPropertiesType = { + "parameters": { + "service_account_info": "", + "catalog": {}, + }, + "catalog": {}, } errors = GSheetsEngineSpec.validate_parameters(properties) assert errors == [ @@ -74,14 +99,12 @@ def test_validate_parameters_catalog( ] properties: GSheetsPropertiesType = { - "parameters": { - "service_account_info": "", - "catalog": { - "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", - "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", - "not_a_sheet": "https://www.google.com/", - }, - } + "parameters": {"service_account_info": "", "catalog": None}, + "catalog": { + "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", + "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", + "not_a_sheet": "https://www.google.com/", + }, } errors = GSheetsEngineSpec.validate_parameters(properties) # ignore: type @@ -168,12 +191,13 @@ def test_validate_parameters_catalog_and_credentials( properties: GSheetsPropertiesType = { "parameters": { "service_account_info": "", - "catalog": { - "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", - "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", - "not_a_sheet": "https://www.google.com/", - }, - } + "catalog": None, + }, + "catalog": { + "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", + "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", + "not_a_sheet": "https://www.google.com/", + }, } errors = GSheetsEngineSpec.validate_parameters(properties) # ignore: type assert errors == [