Database Filtering:

- Add more info to our comments in our tests

tests/integration_tests/databases/api_tests.py

@@ -3637,7 +3637,11 @@ def test_validate_sql_endpoint_failure(self, get_validator_by_name):
 
     def test_get_databases_with_extra_filters(self):
         """
-        API: Test get database with extra query filter
+        API: Test get database with extra query filter.
+        Here we are testing our default where all databases
+        must be returned if nothing is being set in the config.
+        Then, we're adding the patch for the config to add the filter function
+        and testing it's being applied.
         """
         self.login(username="admin")
         extra = {
@@ -3650,6 +3654,7 @@ def test_get_databases_with_extra_filters(self):
 
         if example_db.backend == "sqlite":
             return
+        # Create our two databases
         database_data = {
             "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
             "configuration_method": ConfigurationMethod.SQLALCHEMY_FORM,
@@ -3671,6 +3676,7 @@ def test_get_databases_with_extra_filters(self):
         second_response = json.loads(rv.data.decode("utf-8"))
         self.assertEqual(rv.status_code, 201)
 
+        # The filter function
         def _base_filter(query):
             from superset.models.core import Database
 
@@ -3683,28 +3689,34 @@ def _base_filter(query):
         expected_names.sort()
 
         uri = f"api/v1/database/"
+        # Get the list of databases without filter in the config
         rv = self.client.get(uri)
         data = json.loads(rv.data.decode("utf-8"))
+        # All databases must be returned if no filter is present
         self.assertEqual(data["count"], len(dbs))
         database_names = [item["database_name"] for item in data["result"]]
         database_names.sort()
         # All Databases because we are an admin
         self.assertEqual(database_names, expected_names)
         assert rv.status_code == 200
+        # Our filter function wasn't get called
         base_filter_mock.assert_not_called()
 
+        # Now we patch the config to include our filter function
         with patch.dict(
             "superset.views.filters.current_app.config",
             {"EXTRA_DYNAMIC_QUERY_FILTERS": {"databases": base_filter_mock}},
         ):
             uri = f"api/v1/database/"
             rv = self.client.get(uri)
             data = json.loads(rv.data.decode("utf-8"))
+            # Only one database start with dyntest
             self.assertEqual(data["count"], 1)
             database_names = [item["database_name"] for item in data["result"]]
             # Only the database that starts with tests, even if we are an admin
             self.assertEqual(database_names, ["dyntest-create-database-1"])
             assert rv.status_code == 200
+            # The filter function is called now that it's defined in our config
             base_filter_mock.assert_called()
 
         # Cleanup

tests/unit_tests/databases/api_test.py

@@ -507,7 +507,10 @@ def test_apply_dynamic_database_filter(
     full_api_access: None,
 ) -> None:
     """
-    Test that we can filter the list of databases
+    Test that we can filter the list of databases.
+    First test the default behavior without a filter and then
+    defining a filter function and patching the config to get
+    the filtered results.
     """
     with app.app_context():
         from superset.daos.database import DatabaseDAO