Address review feedback

Author: aclark4life

django_mongodb_extensions/mql_panel/forms.py

@@ -110,7 +110,7 @@ def _execute_query(self, db, collection, collection_name, operation, args_list):
 
     def _flatten_single_key_dicts(self, obj):
         if isinstance(obj, dict):
-            if len(obj) == 1:
+            if len(obj) == 1 and next(iter(obj)).startswith("$"):
                 only_value = next(iter(obj.values()))
                 return self._flatten_single_key_dicts(only_value)
             return {

django_mongodb_extensions/mql_panel/panel.py

@@ -81,17 +81,20 @@ def content(self):
 
     def disable_instrumentation(self):
         for connection in connections.all():
-            if hasattr(connection, "_djdt_logger"):
-                connection._djdt_logger = None
+            if hasattr(connection, "_mql_djdt_logger"):
+                connection._mql_djdt_logger = None
 
     def enable_instrumentation(self):
         # Only patch MongoDB connections (those with get_collection method).
         # This allows the panel to work in multi-database setups with
         # both MongoDB and relational databases.
+        #
+        # Use _mql_djdt_logger (not _djdt_logger) to avoid conflicting with the
+        # SQL panel, which sets _djdt_logger on all connections regardless of type.
         for connection in connections.all():
             if hasattr(connection, "get_collection"):
                 patch_get_collection(connection)
-                connection._djdt_logger = self
+                connection._mql_djdt_logger = self
 
     def generate_stats(self, request, response):
         duplicate_query_groups = defaultdict(list)

django_mongodb_extensions/mql_panel/utils.py

@@ -85,7 +85,9 @@ def patch_get_collection(connection):
         connection._original_get_collection = connection.get_collection
 
     def get_collection(self, name, **kwargs):
-        logger = getattr(self, "_djdt_logger", None)
+        # Use _mql_djdt_logger (not _djdt_logger) to avoid conflicting with the
+        # SQL panel, which sets _djdt_logger on all connections.
+        logger = getattr(self, "_mql_djdt_logger", None)
         if logger:
             collection = self._original_get_collection(name, **kwargs)
             return DebugToolbarWrapper(self, collection, logger)

django_mongodb_extensions/templates/mql_panel/mql_query.html

@@ -34,21 +34,7 @@ <h4>{% translate "Query Results" %}</h4>
                       <tr>
                         <th>{{ sub.key }}</th>
                         <td>
-                          {% if sub.type == "dict" %}
-                            <table class="subtable">
-                              {% for inner in sub.value %}
-                                <tr><th>{{ inner.key }}</th><td>{{ inner.value }}</td></tr>
-                              {% endfor %}
-                            </table>
-                          {% elif sub.type == "list" %}
-                            <ul>
-                              {% for inner in sub.value %}
-                                <li>{{ inner.value }}</li>
-                              {% endfor %}
-                            </ul>
-                          {% else %}
-                            {{ sub.value }}
-                          {% endif %}
+                          {% include 'mql_panel/subtype_value.html' %}
                         </td>
                       </tr>
                     {% endfor %}
@@ -57,21 +43,7 @@ <h4>{% translate "Query Results" %}</h4>
                   <ul>
                     {% for sub in column.value %}
                       <li>
-                        {% if sub.type == "dict" %}
-                          <table class="subtable">
-                            {% for inner in sub.value %}
-                              <tr><th>{{ inner.key }}</th><td>{{ inner.value }}</td></tr>
-                            {% endfor %}
-                          </table>
-                        {% elif sub.type == "list" %}
-                          <ul>
-                            {% for inner in sub.value %}
-                              <li>{{ inner.value }}</li>
-                            {% endfor %}
-                          </ul>
-                        {% else %}
-                          {{ sub.value }}
-                        {% endif %}
+                        {% include 'mql_panel/subtype_value.html' %}
                       </li>
                     {% endfor %}
                   </ul>

django_mongodb_extensions/templates/mql_panel/subtype_value.html

@@ -0,0 +1,30 @@
+{% if sub.type == "dict" %}
+  <table class="subtable">
+    {% for inner in sub.value %}
+      <tr>
+        <th>{{ inner.key }}</th>
+        <td>
+          {% if inner.type %}
+            {% include "mql_panel/subtype_value.html" with sub=inner %}
+          {% else %}
+            {{ inner.value }}
+          {% endif %}
+        </td>
+      </tr>
+    {% endfor %}
+  </table>
+{% elif sub.type == "list" %}
+  <ul>
+    {% for inner in sub.value %}
+      <li>
+        {% if inner.type %}
+          {% include "mql_panel/subtype_value.html" with sub=inner %}
+        {% else %}
+          {{ inner.value }}
+        {% endif %}
+      </li>
+    {% endfor %}
+  </ul>
+{% else %}
+  {{ sub.value }}
+{% endif %}

tests/mql_panel/test_forms.py

@@ -1,7 +1,6 @@
 import datetime
-import json
 
-from bson import ObjectId, json_util
+from bson import ObjectId
 from django.test import TestCase
 
 from django_mongodb_extensions.mql_panel.forms import MQLQueryForm
@@ -12,13 +11,11 @@ def setUp(self):
         self.form = MQLQueryForm()
 
     def test_empty_documents(self):
-        """Empty document list returns empty rows and headers."""
         rows, headers = self.form.convert_documents_to_table([])
         self.assertEqual(rows, [])
         self.assertEqual(headers, [])
 
     def test_simple_fields(self):
-        """Primitive field values like in model query output."""
         cases = [
             ("username", {"value": "username", "is_json": False}),
             (42, {"value": "42", "is_json": False}),
@@ -30,20 +27,16 @@ def test_simple_fields(self):
                 self.assertEqual(self.form._format_cell_value(input_value), expected)
 
     def test_objectid(self):
-        """MongoDB ObjectId from a document."""
-        oid = "69cd52da72ad0703d3dfef51"
+        oid = ObjectId("69cd52da72ad0703d3dfef51")
         result = self.form._format_cell_value(oid)
-        # ObjectIds serialize to {"$oid": "<id>"} which is single-key
         self.assertIs(result["is_json"], False)
-        self.assertEqual(result["value"], oid)
+        self.assertEqual(result["value"], "69cd52da72ad0703d3dfef51")
 
     def test_datetime(self):
-        """Datetime values as they might appear in a MongoDB doc."""
         dt = datetime.datetime(2024, 1, 1, 12, 30)
         result = self.form._format_cell_value(dt)
-        # Datetimes serialize to {"$date": timestamp_ms} which is single-key dict
         self.assertIs(result["is_json"], False)
-        self.assertEqual(result["value"], str(json.loads(json_util.dumps(dt))["$date"]))
+        self.assertEqual(result["value"], "2024-01-01T12:30:00Z")
 
     def test_embedded_document(self):
         embedded_doc = {
@@ -69,6 +62,71 @@ def test_list_field(self):
         value_map = {item["key"]: item["value"] for item in result["value"]}
         self.assertEqual(value_map[0], "tag1")
         self.assertEqual(value_map[1], "tag2")
-        self.assertEqual(value_map[2], "69cd51ddf1a98c14c906c51e")  # ObjectId string
+        self.assertEqual(value_map[2], "69cd51ddf1a98c14c906c51e")
         for item in result["value"]:
             self.assertIs(item["is_json"], False)
+
+    def test_list_of_embedded_dicts(self):
+        tags = [
+            {"name": "cool_tag", "number": 42},
+            {"name": "other_tag", "number": 7},
+        ]
+        result = self.form._format_cell_value(tags)
+        self.assertEqual(result["type"], "list")
+        self.assertIs(result["is_json"], False)
+        for item in result["value"]:
+            self.assertEqual(item["type"], "dict")
+            self.assertIs(item["is_json"], False)
+            inner_map = {inner["key"]: inner["value"] for inner in item["value"]}
+            self.assertIn("name", inner_map)
+            self.assertIn("number", inner_map)
+
+    def test_dict_with_nested_list_of_dicts(self):
+        address = {
+            "street": "123 Main St",
+            "tags": [
+                {"name": "cool_tag", "number": 42},
+            ],
+        }
+        result = self.form._format_cell_value(address)
+        self.assertEqual(result["type"], "dict")
+        self.assertIs(result["is_json"], False)
+        items_by_key = {item["key"]: item for item in result["value"]}
+        self.assertEqual(items_by_key["street"]["value"], "123 Main St")
+        tags_item = items_by_key["tags"]
+        self.assertEqual(tags_item["type"], "list")
+        self.assertIs(tags_item["is_json"], False)
+        first_tag = tags_item["value"][0]
+        self.assertEqual(first_tag["type"], "dict")
+        inner_map = {inner["key"]: inner["value"] for inner in first_tag["value"]}
+        self.assertEqual(inner_map["name"], "cool_tag")
+        self.assertEqual(inner_map["number"], "42")
+
+    def test_deeply_nested_embedded_document(self):
+        address = {
+            "street": "123 Main St",
+            "tags": [
+                {
+                    "name": "cool_tag",
+                    "number": 42,
+                    "person": {"name": "Alice"},
+                }
+            ],
+        }
+        result = self.form._format_cell_value(address)
+        self.assertEqual(result["type"], "dict")
+        address_by_key = {item["key"]: item for item in result["value"]}
+        tags_item = address_by_key["tags"]
+        self.assertEqual(tags_item["type"], "list")
+        self.assertIs(tags_item["is_json"], False)
+        first_tag = tags_item["value"][0]
+        self.assertEqual(first_tag["type"], "dict")
+        self.assertIs(first_tag["is_json"], False)
+        tag_by_key = {inner["key"]: inner for inner in first_tag["value"]}
+        self.assertEqual(tag_by_key["name"]["value"], "cool_tag")
+        self.assertEqual(tag_by_key["number"]["value"], "42")
+        person_item = tag_by_key["person"]
+        self.assertEqual(person_item["type"], "dict")
+        self.assertIs(person_item["is_json"], False)
+        person_map = {inner["key"]: inner["value"] for inner in person_item["value"]}
+        self.assertEqual(person_map["name"], "Alice")

tests/mql_panel/test_panel.py

@@ -5,7 +5,7 @@
 
 from django_mongodb_extensions.mql_panel import MQLPanel
 
-rf = RequestFactory()
+request_factory = RequestFactory()
 
 
 def mql_call():
@@ -18,7 +18,7 @@ class MQLPanelTests(TestCase):
 
     def setUp(self):
         self._get_response = lambda request: HttpResponse()
-        self.request = rf.get("/")
+        self.request = request_factory.get("/")
         self.toolbar = DebugToolbar(self.request, self.get_response)
         self.toolbar.stats = {}
         self.panel = self.toolbar.get_panel_by_id(self.panel_id)