Merge pull request #1484 from ashwch/history_refresh_fix

matthiask · web-flow · commit 3587052329a5 · 2021-07-20T22:41:37.000+02:00
Fixes and improvements to history views
diff --git a/debug_toolbar/panels/history/views.py b/debug_toolbar/panels/history/views.py
@@ -2,6 +2,7 @@
 from django.template.loader import render_to_string
 
 from debug_toolbar.decorators import require_show_toolbar, signed_data_view
+from debug_toolbar.forms import SignedDataForm
 from debug_toolbar.panels.history.forms import HistoryStoreForm
 from debug_toolbar.toolbar import DebugToolbar
 
@@ -16,6 +17,10 @@ def history_sidebar(request, verified_data):
         store_id = form.cleaned_data["store_id"]
         toolbar = DebugToolbar.fetch(store_id)
         context = {}
+        if toolbar is None:
+            # When the store_id has been popped already due to
+            # RESULTS_CACHE_SIZE
+            return JsonResponse(context)
         for panel in toolbar.panels:
             if not panel.is_historical:
                 continue
@@ -40,7 +45,8 @@ def history_refresh(request, verified_data):
 
     if form.is_valid():
         requests = []
-        for id, toolbar in reversed(DebugToolbar._store.items()):
+        # Convert to list to handle mutations happenening in parallel
+        for id, toolbar in list(DebugToolbar._store.items())[::-1]:
             requests.append(
                 {
                     "id": id,
@@ -50,7 +56,11 @@ def history_refresh(request, verified_data):
                             "id": id,
                             "store_context": {
                                 "toolbar": toolbar,
-                                "form": HistoryStoreForm(initial={"store_id": id}),
+                                "form": SignedDataForm(
+                                    initial=HistoryStoreForm(
+                                        initial={"store_id": id}
+                                    ).initial
+                                ),
                             },
                         },
                     ),
diff --git a/debug_toolbar/static/debug_toolbar/js/history.js b/debug_toolbar/static/debug_toolbar/js/history.js
@@ -7,34 +7,45 @@ $$.on(djDebug, "click", ".switchHistory", function (event) {
     const newStoreId = this.dataset.storeId;
     const tbody = this.closest("tbody");
 
-    tbody
-        .querySelector(".djdt-highlighted")
-        .classList.remove("djdt-highlighted");
+    const highlighted = tbody.querySelector(".djdt-highlighted");
+    if (highlighted) {
+        highlighted.classList.remove("djdt-highlighted");
+    }
     this.closest("tr").classList.add("djdt-highlighted");
 
     ajaxForm(this).then(function (data) {
         djDebug.setAttribute("data-store-id", newStoreId);
-        Object.keys(data).forEach(function (panelId) {
-            const panel = document.getElementById(panelId);
-            if (panel) {
-                panel.outerHTML = data[panelId].content;
-                document.getElementById("djdt-" + panelId).outerHTML =
-                    data[panelId].button;
-            }
-        });
+        // Check if response is empty, it could be due to an expired store_id.
+        if (Object.keys(data).length === 0) {
+            const container = document.getElementById("djdtHistoryRequests");
+            container.querySelector(
+                'button[data-store-id="' + newStoreId + '"]'
+            ).innerHTML = "Switch [EXPIRED]";
+        } else {
+            Object.keys(data).forEach(function (panelId) {
+                const panel = document.getElementById(panelId);
+                if (panel) {
+                    panel.outerHTML = data[panelId].content;
+                    document.getElementById("djdt-" + panelId).outerHTML =
+                        data[panelId].button;
+                }
+            });
+        }
     });
 });
 
 $$.on(djDebug, "click", ".refreshHistory", function (event) {
     event.preventDefault();
     const container = document.getElementById("djdtHistoryRequests");
     ajaxForm(this).then(function (data) {
+        // Remove existing rows first then re-populate with new data
+        container
+            .querySelectorAll("tr[data-store-id]")
+            .forEach(function (node) {
+                node.remove();
+            });
         data.requests.forEach(function (request) {
-            if (
-                !container.querySelector('[data-store-id="' + request.id + '"]')
-            ) {
-                container.innerHTML = request.content + container.innerHTML;
-            }
+            container.innerHTML = request.content + container.innerHTML;
         });
     });
 });
diff --git a/tests/panels/test_history.py b/tests/panels/test_history.py
@@ -1,3 +1,5 @@
+import html
+
 from django.test import RequestFactory, override_settings
 from django.urls import resolve, reverse
 
@@ -64,6 +66,21 @@ def test_urls(self):
 
 @override_settings(DEBUG=True)
 class HistoryViewsTestCase(IntegrationTestCase):
+    PANEL_KEYS = {
+        "VersionsPanel",
+        "TimerPanel",
+        "SettingsPanel",
+        "HeadersPanel",
+        "RequestPanel",
+        "SQLPanel",
+        "StaticFilesPanel",
+        "TemplatesPanel",
+        "CachePanel",
+        "SignalsPanel",
+        "LoggingPanel",
+        "ProfilingPanel",
+    }
+
     def test_history_panel_integration_content(self):
         """Verify the history panel's content renders properly.."""
         self.assertEqual(len(DebugToolbar._store), 0)
@@ -88,26 +105,45 @@ def test_history_sidebar_invalid(self):
     def test_history_sidebar(self):
         """Validate the history sidebar view."""
         self.client.get("/json_view/")
-        store_id = list(DebugToolbar._store.keys())[0]
+        store_id = list(DebugToolbar._store)[0]
+        data = {"signed": SignedDataForm.sign({"store_id": store_id})}
+        response = self.client.get(reverse("djdt:history_sidebar"), data=data)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            set(response.json()),
+            self.PANEL_KEYS,
+        )
+
+    @override_settings(
+        DEBUG_TOOLBAR_CONFIG={"RESULTS_CACHE_SIZE": 1, "RENDER_PANELS": False}
+    )
+    def test_history_sidebar_expired_store_id(self):
+        """Validate the history sidebar view."""
+        self.client.get("/json_view/")
+        store_id = list(DebugToolbar._store)[0]
+        data = {"signed": SignedDataForm.sign({"store_id": store_id})}
+        response = self.client.get(reverse("djdt:history_sidebar"), data=data)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            set(response.json()),
+            self.PANEL_KEYS,
+        )
+        self.client.get("/json_view/")
+
+        # Querying old store_id should return in empty response
         data = {"signed": SignedDataForm.sign({"store_id": store_id})}
         response = self.client.get(reverse("djdt:history_sidebar"), data=data)
         self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.json(), {})
+
+        # Querying with latest store_id
+        latest_store_id = list(DebugToolbar._store)[0]
+        data = {"signed": SignedDataForm.sign({"store_id": latest_store_id})}
+        response = self.client.get(reverse("djdt:history_sidebar"), data=data)
+        self.assertEqual(response.status_code, 200)
         self.assertEqual(
-            set(response.json().keys()),
-            {
-                "VersionsPanel",
-                "TimerPanel",
-                "SettingsPanel",
-                "HeadersPanel",
-                "RequestPanel",
-                "SQLPanel",
-                "StaticFilesPanel",
-                "TemplatesPanel",
-                "CachePanel",
-                "SignalsPanel",
-                "LoggingPanel",
-                "ProfilingPanel",
-            },
+            set(response.json()),
+            self.PANEL_KEYS,
         )
 
     def test_history_refresh_invalid_signature(self):
@@ -128,5 +164,10 @@ def test_history_refresh(self):
         self.assertEqual(response.status_code, 200)
         data = response.json()
         self.assertEqual(len(data["requests"]), 1)
+
+        store_id = list(DebugToolbar._store)[0]
+        signature = SignedDataForm.sign({"store_id": store_id})
+        self.assertIn(html.escape(signature), data["requests"][0]["content"])
+
         for val in ["foo", "bar"]:
             self.assertIn(val, data["requests"][0]["content"])
