fix: gsheets editing with dynamic forms (#21710)

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -569,6 +569,18 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     // 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<DatabaseModalProps> = ({
       });
     }
 
-    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 = () => (

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,

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,16 +216,22 @@ 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,
         # at all other times they are a dict
         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:

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 == [