Address review feedback

Author: aclark4life

django_mongodb_extensions/mql_panel/forms.py

@@ -9,47 +9,26 @@
 from django.utils.translation import gettext_lazy as _
 from pymongo import errors as pymongo_errors
 
-from django_mongodb_extensions.mql_panel.utils import (
-    QueryParts,
+from .utils import (
     get_max_query_results,
     parse_query_args,
 )
 
 
 class MQLBaseForm(SQLSelectForm):
     def _execute_operation(self, operation_type, executor_func):
-        mql_string = ""
-        try:
-            parts = self._get_query_parts()
-            mql_string = parts.mql_string
-            return executor_func(
-                parts.db,
-                parts.collection,
-                parts.collection_name,
-                parts.operation,
-                parts.args_list,
-            )
-        except (ValueError, pymongo_errors.PyMongoError) as e:
-            # ValueError: unsupported operation or unserializable args.
-            # PyMongoError: any MongoDB driver error during execution.
-            return self._handle_operation_error(e, mql_string, operation_type)
-
-    def _get_query_parts(self):
         query_dict = self.cleaned_data["query"]
         alias = query_dict.get("alias", "default")
         connection = connections[alias]
-        collection_name, operation, args_list = parse_query_args(query_dict)
         db = connection.database
-        return QueryParts(
-            query_dict=query_dict,
-            alias=alias,
-            mql_string=query_dict.get("mql", ""),
-            connection=connection,
-            db=db,
-            collection=db[collection_name],
-            collection_name=collection_name,
-            operation=operation,
-            args_list=args_list,
+        collection_name, operation, args_list = parse_query_args(query_dict)
+        collection = db[collection_name]
+        return executor_func(
+            db,
+            collection,
+            collection_name,
+            operation,
+            args_list,
         )
 
     def _handle_operation_error(self, error, mql_string, operation_type="operation"):
@@ -193,34 +172,58 @@ def _execute_query(self, db, collection, collection_name, operation, args_list):
             raise ValueError(f"Unsupported read operation: {operation}")
         return self.convert_documents_to_table(result_docs)
 
+    def _flatten_single_key_dicts(self, obj):
+        if isinstance(obj, dict):
+            if len(obj) == 1:
+                only_value = next(iter(obj.values()))
+                return self._flatten_single_key_dicts(only_value)
+            return {
+                key_name: self._flatten_single_key_dicts(value_item)
+                for key_name, value_item in obj.items()
+            }
+        elif isinstance(obj, list):
+            return [self._flatten_single_key_dicts(value_item) for value_item in obj]
+        return obj
+
     def _format_cell_value(self, value):
-        """Format a single cell value for table display."""
-        if value is None:
-            return {"value": "", "is_json": False}
-        # Handle primitive types directly without JSON serialization
-        if isinstance(value, (str, int, float, bool)):
-            return {"value": str(value), "is_json": False}
-        # For complex types (ObjectId, datetime, dicts, lists, etc.), use json_util
-        try:
-            serialized = json_util.dumps(value)
-        except (TypeError, AttributeError):
-            return {"value": str(value), "is_json": False}
-        try:
-            parsed = json.loads(serialized)
-        except json.JSONDecodeError:
-            return {"value": serialized, "is_json": False}
-        # Extract value from single-key BSON extended JSON objects like {"$oid": "..."}
-        if isinstance(parsed, dict) and len(parsed) == 1:
-            key, val = next(iter(parsed.items()))
-            return {"value": str(val), "is_json": False}
-        # For multi-key objects, format with indentation for readability
-        if isinstance(parsed, dict) and len(parsed) > 1:
-            return {"value": json.dumps(parsed, indent=4), "is_json": True}
-        # For lists and other types, use compact serialization
-        return {"value": serialized, "is_json": False}
+        serialized = json_util.dumps(value)
+        parsed_json = json.loads(serialized)
+        flattened_value = self._flatten_single_key_dicts(parsed_json)
+        if isinstance(flattened_value, (str, int, float, bool)):
+            return {"value": str(flattened_value), "is_json": False}
+        if isinstance(flattened_value, dict):
+            return {
+                "type": "dict",
+                "value": self._format_dict_for_template(flattened_value),
+                "is_json": False,
+            }
+        if isinstance(flattened_value, list):
+            return {
+                "type": "list",
+                "value": self._format_list_for_template(flattened_value),
+                "is_json": False,
+            }
+        return {
+            "value": json.dumps(flattened_value, indent=4),
+            "is_json": True,
+        }
+
+    def _format_dict_for_template(self, dictionary):
+        return [
+            {"key": key_name, **self._format_cell_value(value_item)}
+            for key_name, value_item in dictionary.items()
+        ]
+
+    def _format_list_for_template(self, list_items):
+        return [
+            {"key": index, **self._format_cell_value(value_item)}
+            for index, value_item in enumerate(list_items)
+        ]
+
+    def _format_row(self, row_dict):
+        return [self._format_cell_value(cell_value) for cell_value in row_dict.values()]
 
     def convert_documents_to_table(self, documents):
-        """Convert MongoDB documents to a table of rows and headers."""
         if not documents:
             return [], []
         # Collect all unique field names

django_mongodb_extensions/mql_panel/panel.py

@@ -14,8 +14,8 @@
 from django.utils.translation import gettext_lazy as _
 from django.utils.translation import ngettext
 
-from django_mongodb_extensions.mql_panel import views
-from django_mongodb_extensions.mql_panel.utils import (
+from . import views
+from .utils import (
     get_mql_warning_threshold,
     patch_get_collection,
     patch_new_connection,

django_mongodb_extensions/mql_panel/utils.py

@@ -36,39 +36,9 @@ def log(self, op, duration, args, kwargs=None):
             )
 
 
-class QueryParts:
-    """Structured container for parsed query components."""
-
-    def __init__(
-        self,
-        query_dict,
-        alias,
-        mql_string,
-        connection,
-        db,
-        collection,
-        collection_name,
-        operation,
-        args_list,
-    ):
-        self.query_dict = query_dict
-        self.alias = alias
-        self.mql_string = mql_string
-        self.connection = connection
-        self.db = db
-        self.collection = collection
-        self.collection_name = collection_name
-        self.operation = operation
-        self.args_list = args_list
-
-
 def format_mql_query(query):
     """Return a pretty-printed MQL query string."""
-    mql_string = query.get("mql", "")
-    try:
-        collection_name, operation, args_list = parse_query_args(query)
-    except ValueError:
-        return mql_string
+    collection_name, operation, args_list = parse_query_args(query)
     if args_list:
         args_formatted = json_util.dumps(
             args_list[0] if len(args_list) == 1 else args_list,

django_mongodb_extensions/templates/mql_panel/mql_query.html

@@ -13,6 +13,7 @@ <h3>{% translate "Query Results" %}</h3>
       <dt>{% translate "Database" %}</dt>
       <dd>{{ alias }}</dd>
     </dl>
+
     <h4>{% translate "Query Results" %}</h4>
     <table>
       <thead>
@@ -27,10 +28,57 @@ <h4>{% translate "Query Results" %}</h4>
           <tr>
             {% for column in row %}
               <td>
-                {% if column.is_json %}
-                    <pre style="margin: 0; padding: 0; border: none; background: none; white-space: pre-wrap; word-wrap: break-word; font-family: inherit;">{{ column.value|escape }}</pre>
+                {% if column.type == "dict" %}
+                  <table class="subtable">
+                    {% for sub in column.value %}
+                      <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 %}
+                        </td>
+                      </tr>
+                    {% endfor %}
+                  </table>
+                {% elif column.type == "list" %}
+                  <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 %}
+                      </li>
+                    {% endfor %}
+                  </ul>
+                {% elif column.is_json %}
+                  <pre style="margin: 0; padding: 0; border: none; background: none; white-space: pre-wrap; word-wrap: break-word; font-family: inherit;">{{ column.value }}</pre>
                 {% else %}
-                  {{ column.value|escape }}
+                  {{ column.value }}
                 {% endif %}
               </td>
             {% endfor %}

tests/mql_panel/test_forms.py

@@ -1,3 +1,7 @@
+import datetime
+import json
+
+from bson import ObjectId, json_util
 from django.test import TestCase
 
 from django_mongodb_extensions.mql_panel.forms import MQLQueryForm
@@ -32,3 +36,61 @@ def test_handle_operation_error_format(self):
 
         # Should have one header
         self.assertEqual(headers[0], "Query Parsing Error")
+
+
+class FormatCellValueTests(TestCase):
+    def setUp(self):
+        self.form = MQLQueryForm()
+
+    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}),
+            (3.14, {"value": "3.14", "is_json": False}),
+            (True, {"value": "True", "is_json": False}),
+        ]
+        for input_value, expected in cases:
+            with self.subTest(value=input_value):
+                self.assertEqual(self.form._format_cell_value(input_value), expected)
+
+    def test_objectid(self):
+        """MongoDB ObjectId from a document."""
+        oid = ObjectId()
+        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"], str(json.loads(json_util.dumps(oid))["$oid"]))
+
+    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"]))
+
+    def test_embedded_document(self):
+        """Nested dict with multiple keys (JSON formatting)."""
+        embedded_doc = {
+            "name": "Alice",
+            "age": 30,
+            "created": datetime.datetime(2024, 2, 1, 18, 0),
+        }
+        result = self.form._format_cell_value(embedded_doc)
+        self.assertIs(result["is_json"], False)
+        parsed_back = json.loads(result["value"])
+        # Datetime will still be JSON date dict
+        self.assertEqual(parsed_back["name"], "Alice")
+        self.assertEqual(parsed_back["age"], 30)
+        self.assertIn("$date", parsed_back["created"])
+
+    def test_list_field(self):
+        """A list of values as might appear in MongoDB array field."""
+        arr = ["tag1", "tag2", ObjectId()]
+        result = self.form._format_cell_value(arr)
+        # Lists are not dicts, so is_json = False
+        self.assertIs(result["is_json"], False)
+        self.assertEqual(
+            result["value"], '["tag1", "tag2", {"$oid": "69cc8219f859271f0a081538"}]'
+        )