From bdb0b25152bdc412bcc523c2dad2cd1261c8f70a Mon Sep 17 00:00:00 2001
From: Jonas Maison <Jonas1312@users.noreply.github.com>
Date: Fri, 29 Sep 2023 10:46:31 +0200
Subject: [PATCH] fix: remove null from variables in gql query

---
 src/kili/core/graphql/graphql_client.py | 21 ++-----
 tests/unit/test_graphql_client.py       | 73 +++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/src/kili/core/graphql/graphql_client.py b/src/kili/core/graphql/graphql_client.py
index 100801ffc..a80120349 100644
--- a/src/kili/core/graphql/graphql_client.py
+++ b/src/kili/core/graphql/graphql_client.py
@@ -234,22 +234,9 @@ def _get_kili_app_version(self) -> Optional[str]:
         return None
 
     @classmethod
-    def _remove_keys_with_none_values(cls, variables: Dict) -> Dict:
-        """Remove keys with None values from a nested dictionary."""
-        output_dict = {}
-        for key, value in variables.items():
-            if value is None:
-                continue
-
-            new_value = value
-            if isinstance(value, dict):
-                dict_without_nones = cls._remove_keys_with_none_values(value)
-                if len(dict_without_nones):
-                    new_value = dict_without_nones
-
-            output_dict[key] = new_value
-
-        return output_dict
+    def _remove_nullable_inputs(cls, variables: Dict) -> Dict:
+        """Remove nullable inputs from the variables."""
+        return {k: v for k, v in variables.items() if v is not None}
 
     def execute(
         self, query: Union[str, DocumentNode], variables: Optional[Dict] = None, **kwargs
@@ -262,7 +249,7 @@ def execute(
             kwargs: additional arguments to pass to the GraphQL client
         """
         document = query if isinstance(query, DocumentNode) else gql(query)
-        variables = self._remove_keys_with_none_values(variables) if variables else None
+        variables = self._remove_nullable_inputs(variables) if variables else None
 
         try:
             return self._execute_with_retries(document, variables, **kwargs)
diff --git a/tests/unit/test_graphql_client.py b/tests/unit/test_graphql_client.py
index 160fa7272..7dae426df 100644
--- a/tests/unit/test_graphql_client.py
+++ b/tests/unit/test_graphql_client.py
@@ -2,6 +2,7 @@
 from pathlib import Path
 from tempfile import TemporaryDirectory
 from time import time
+from typing import Dict
 from unittest import mock
 
 import graphql
@@ -355,3 +356,75 @@ def mocked_backend_response(*args, **kwargs):
     # Then
     assert result["data"] == "all good"
     assert mocked_execute.call_count == nb_times_called == 3
+
+
+@pytest.mark.parametrize(
+    ("variables", "expected"),
+    [
+        ({"id": "123456"}, {"id": "123456"}),
+        ({"id": None}, {}),
+        (
+            {
+                "project": {"id": "project_id"},
+                "asset": {"id": None},
+                "assetIn": ["123456"],
+                "status": "some_status",
+                "type": None,
+            },
+            {
+                "project": {"id": "project_id"},
+                "asset": {"id": None},
+                "assetIn": ["123456"],
+                "status": "some_status",
+            },
+        ),
+        (
+            {
+                "id": None,
+                "searchQuery": "truc",
+                "shouldRelaunchKpiComputation": None,
+                "starred": True,
+                "updatedAtGte": None,
+                "updatedAtLte": None,
+                "createdAtGte": None,
+                "createdAtLte": None,
+                "tagIds": ["tag_id"],
+            },
+            {
+                "searchQuery": "truc",
+                "starred": True,
+                "tagIds": ["tag_id"],
+            },
+        ),
+        (  # assetwhere
+            {
+                "externalIdStrictlyIn": ["truc"],
+                "externalIdIn": None,
+                "honeypotMarkGte": None,
+                "honeypotMarkLte": 0.0,
+                "id": "fake_asset_id",
+                "metadata": {"key": None},  # this field is a JSON graphql type. It should be kept
+                "project": {"id": "fake_proj_id"},
+                "skipped": True,
+                "updatedAtLte": None,
+            },
+            {
+                "externalIdStrictlyIn": ["truc"],
+                "honeypotMarkLte": 0.0,
+                "id": "fake_asset_id",
+                "metadata": {"key": None},
+                "project": {"id": "fake_proj_id"},
+                "skipped": True,
+            },
+        ),
+    ],
+)
+def test_given_variables_when_i_remove_null_values_then_it_works(variables: Dict, expected: Dict):
+    # Given
+    _ = variables
+
+    # When
+    output = GraphQLClient._remove_nullable_inputs(variables)
+
+    # Then
+    assert output == expected