From dac34ff1779352e83a8150d272870191c36f287d Mon Sep 17 00:00:00 2001
From: Antonio Rivero <antonioriverocode@gmail.com>
Date: Thu, 6 Jul 2023 16:18:05 -0400
Subject: [PATCH] Database Filtering:

- Add more info to our comments in our tests
---
 tests/integration_tests/databases/api_tests.py | 14 +++++++++++++-
 tests/unit_tests/databases/api_test.py         |  5 ++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index 6d6c832abcbc7..ebf94219c3b0a 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/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,16 +3689,20 @@ 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}},
@@ -3700,11 +3710,13 @@ def _base_filter(query):
             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
diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py
index 71c078f10c414..899e2b0234571 100644
--- a/tests/unit_tests/databases/api_test.py
+++ b/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