From 7861ad7990583c37182728b6cf9d6a53a7fa740f Mon Sep 17 00:00:00 2001
From: Chris Clark <chris@untrod.com>
Date: Mon, 22 Apr 2024 08:51:24 -0400
Subject: [PATCH] 4.1.0. Cleaner markup, test urls, logging level, keyboard
 affordances for assistant

---
 HISTORY.rst                                |  10 ++-
 README.rst                                 |   2 +-
 docs/development.rst                       |   2 +-
 explorer/__init__.py                       |   4 +-
 explorer/src/js/assistant.js               | 100 +++++++++++----------
 explorer/src/js/explorer.js                |   1 -
 explorer/templates/explorer/assistant.html |  18 ++--
 explorer/tests/urls.py                     |  14 ---
 explorer/tracker.py                        |   5 +-
 test_project/settings.py                   |   2 +-
 test_project/urls.py                       |  17 ++++
 11 files changed, 99 insertions(+), 76 deletions(-)
 delete mode 100644 explorer/tests/urls.py
 create mode 100644 test_project/urls.py

diff --git a/HISTORY.rst b/HISTORY.rst
index 6fe110d1..e914d79d 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -5,12 +5,14 @@ Change Log
 This document records all notable changes to `django-sql-explorer <https://github.com/chrisclark/django-sql-explorer>`_.
 This project adheres to `Semantic Versioning <https://semver.org/>`_.
 
-`4.1.0b2`_ (2024-04-17)
+`4.1.0`_ (2024-04-23)
 ===========================
 * SQL Assistant: Built in query help via OpenAI (or LLM of choice), with relevant schema
-  automatically injected into the prompt. Enable via setting EXPLORER_AI_API_KEY.
-* Anonymous usage telemetry. Can be disabled by setting EXPLORER_ENABLE_ANONYMOUS_STATS to False
-* `#594`_: Eliminate <script> tags to prevent potential Content Security Policy issues
+  automatically injected into the prompt. Enable by setting EXPLORER_AI_API_KEY.
+* Anonymous usage telemetry. Disable by setting EXPLORER_ENABLE_ANONYMOUS_STATS to False.
+* Refactor pip requirements to make 'extras' more robust and easier to manage.
+* `#592`_: Support user models with no email fields
+* `#594`_: Eliminate <script> tags to prevent potential Content Security Policy issues.
 
 `4.0.2`_ (2024-02-06)
 ===========================
diff --git a/README.rst b/README.rst
index 4f3fa756..aa0f01e6 100644
--- a/README.rst
+++ b/README.rst
@@ -90,4 +90,4 @@ create a new virtualenv, cd into ``test_project`` and run ``start.sh`` (or
 walk through the steps yourself) to get a test instance of the app up
 and running.
 
-You can now navigate to 127.0.0.1:8000/ and begin exploring!
+You can now navigate to 127.0.0.1:8000/explorer/ and begin exploring!
diff --git a/docs/development.rst b/docs/development.rst
index 154cd7d9..ba943e24 100644
--- a/docs/development.rst
+++ b/docs/development.rst
@@ -6,7 +6,7 @@ create a new virtualenv, cd into ``test_project`` and run ``start.sh`` (or
 walk through the steps yourself) to get a test instance of the app up
 and running.
 
-You can now navigate to 127.0.0.1:8000/ and begin exploring!
+You can now navigate to 127.0.0.1:8000/explorer/ and begin exploring!
 
 Installing From Source
 ----------------------
diff --git a/explorer/__init__.py b/explorer/__init__.py
index 1be7efe7..2848706f 100644
--- a/explorer/__init__.py
+++ b/explorer/__init__.py
@@ -2,8 +2,8 @@
     "major": 4,
     "minor": 1,
     "patch": 0,
-    "releaselevel": "beta",
-    "serial": 5
+    "releaselevel": "final",
+    "serial": 0
 }
 
 
diff --git a/explorer/src/js/assistant.js b/explorer/src/js/assistant.js
index e65f2809..b826581d 100644
--- a/explorer/src/js/assistant.js
+++ b/explorer/src/js/assistant.js
@@ -5,13 +5,15 @@ import * as bootstrap from 'bootstrap';
 import $ from "jquery";
 import List from "list.js";
 
-const spinner = "<div class=\"spinner-border text-primary\" role=\"status\"><span class=\"visually-hidden\">Loading...</span></div>";
-
 function getErrorMessage() {
     const errorElement = document.querySelector('.alert-danger.db-error');
     return errorElement ? errorElement.textContent.trim() : null;
 }
 
+function assistantSearchFocus() {
+    document.getElementById("search_assistant_tables").focus();
+}
+
 export function setUpAssistant(expand = false) {
 
     const error = getErrorMessage();
@@ -23,12 +25,7 @@ export function setUpAssistant(expand = false) {
         });
         bsCollapse.show();
         if(error) {
-            const textarea = document.getElementById('id_assistant_input');
-            textarea.value = "Please help me fix the error(s) in this query.";
-            const newDiv = document.createElement('div');
-            newDiv.textContent = 'Error messages are automatically included in the prompt. Just hit "Ask Assistant" and all relevant context will be injected to the LLM request.';  // Add any text or HTML content
-            newDiv.className = 'text-secondary small';
-            textarea.parentNode.insertBefore(newDiv, textarea.nextSibling);
+            document.getElementById('id_error_help_message').classList.remove('d-none');
         }
     }
 
@@ -83,6 +80,7 @@ export function setUpAssistant(expand = false) {
             additionalTableContainer.classList.remove('d-none');
             assistantInputWrapper.classList.remove('col-12');
             assistantInputWrapper.classList.add('col-9');
+            assistantSearchFocus();
         } else {
             additionalTableContainer.classList.add('d-none');
             assistantInputWrapper.classList.remove('col-9');
@@ -94,46 +92,56 @@ export function setUpAssistant(expand = false) {
     });
     showHideExtraTables(checkbox.checked);
 
-    document.getElementById('ask_assistant_btn').addEventListener('click', function() {
-
-        const selectedTables = Array.from(
-            document.querySelectorAll('.table-checkbox:checked')
-        ).map(cb => cb.value);
+    document.getElementById('id_assistant_input').addEventListener('keydown', function(event) {
+        if ((event.ctrlKey || event.metaKey) && (event.key === 'Enter')) {
+            event.preventDefault();
+            submitAssistantAsk();
+        }
+    });
 
-        const data = {
-            sql: window.editor?.state.doc.toString() ?? null,
-            connection: document.getElementById("id_connection")?.value ?? null,
-            assistant_request: document.getElementById("id_assistant_input")?.value ?? null,
-            selected_tables: selectedTables,
-            db_error: getErrorMessage()
-        };
+    document.getElementById('ask_assistant_btn').addEventListener('click', submitAssistantAsk);
+}
 
-        document.getElementById("response_block").style.display = "block";
-        document.getElementById("assistant_response").innerHTML = spinner;
-
-        fetch('../assistant/', {
-            method: 'POST',
-            headers: {
-                'Content-Type': 'application/json',
-                'X-CSRFToken': getCsrfToken()
-            },
-            body: JSON.stringify(data)
-        })
-        .then(response => {
-            if (!response.ok) {
-                throw new Error('Network response was not ok');
-            }
-            return response.json();
-        })
-        .then(data => {
-            const output = DOMPurify.sanitize(marked.parse(data.message));
-            document.getElementById("response_block").style.display = "block";
-            document.getElementById("assistant_response").innerHTML = output;
-            setUpCopyButtons();
-        })
-            .catch(error => {
-            console.error('Error:', error);
-        });
+function submitAssistantAsk() {
+
+    const selectedTables = Array.from(
+        document.querySelectorAll('.table-checkbox:checked')
+    ).map(cb => cb.value);
+
+    const data = {
+        sql: window.editor?.state.doc.toString() ?? null,
+        connection: document.getElementById("id_connection")?.value ?? null,
+        assistant_request: document.getElementById("id_assistant_input")?.value ?? null,
+        selected_tables: selectedTables,
+        db_error: getErrorMessage()
+    };
+
+    document.getElementById("assistant_response").innerHTML = '';
+    document.getElementById("response_block").classList.remove('d-none');
+    document.getElementById("assistant_spinner").classList.remove('d-none');
+
+    fetch('../assistant/', {
+        method: 'POST',
+        headers: {
+            'Content-Type': 'application/json',
+            'X-CSRFToken': getCsrfToken()
+        },
+        body: JSON.stringify(data)
+    })
+    .then(response => {
+        if (!response.ok) {
+            throw new Error('Network response was not ok');
+        }
+        return response.json();
+    })
+    .then(data => {
+        const output = DOMPurify.sanitize(marked.parse(data.message));
+        document.getElementById("assistant_response").innerHTML = output;
+        document.getElementById("assistant_spinner").classList.add('d-none');
+        setUpCopyButtons();
+    })
+        .catch(error => {
+        console.error('Error:', error);
     });
 }
 
diff --git a/explorer/src/js/explorer.js b/explorer/src/js/explorer.js
index 3c1d80bd..5918d8ff 100644
--- a/explorer/src/js/explorer.js
+++ b/explorer/src/js/explorer.js
@@ -40,7 +40,6 @@ export class ExplorerEditor {
         }
 
         this.queryId = queryId;
-        this.$table = $("#preview");
         this.$rows = $("#rows");
         this.$form = $("form");
         this.$snapshotField = $("#id_snapshot");
diff --git a/explorer/templates/explorer/assistant.html b/explorer/templates/explorer/assistant.html
index bfb2ad0f..a3aa0548 100644
--- a/explorer/templates/explorer/assistant.html
+++ b/explorer/templates/explorer/assistant.html
@@ -12,9 +12,12 @@
             <div class="row assistant_input_parent">
                 <div class="mt-3 col-12" id="assistant_input_wrapper">
                     <textarea
-                        class="form-control" id="id_assistant_input"
+                        class="form-control mb-4" id="id_assistant_input"
                         name="sql_assistant" rows="3" placeholder="What do you need help with?"></textarea>
-                    <label for="id_assistant_input" class="form-label" id="id_assistant_input_label"></label>
+                    <label for="id_assistant_input" class="form-label d-none" id="id_assistant_input_label">Assistant prompt</label>
+                    <div id="id_error_help_message" class="d-none text-secondary small">
+                        Hit "Ask Assistant" to try and fix the issue. The assistant is automatically aware of error messages & context.
+                    </div>
                 </div>
                 <div id="additional_table_container" class="d-none col-3">
                     <input id="search_assistant_tables" class="search" placeholder="{% trans "Search Tables" %}" />
@@ -29,7 +32,7 @@
                         <input type="checkbox" class="form-check-input" name="assistant-include-other-tables" id="include_other_tables" autocomplete="off">
                         <label class="form-check-label" for="include_other_tables">Include Other Tables (<a class="small text-secondary" style="cursor: pointer;"
                             data-bs-toggle="tooltip" data-bs-placement="top"
-                            data-bs-title="SQL Assistant builds a prompt with your query, your request, relevant schema (tables referenced in your query) and sample data from those tables. You can optionally include other tables (schema + data sample) that you'd like SQL Assistant to know about.">What is this?</a>)</label>
+                            data-bs-title="SQL Assistant builds a prompt with your query, your request, relevant schema (tables referenced in your query) and sample data from those tables. You can optionally include other tables (schema + data sample) that you'd like SQL Assistant to know about.">?</a>)</label>
                     </div>
                 </div>
                 <div class="col-6 text-end">
@@ -40,8 +43,13 @@
             </div>
 
 
-            <div id="response_block" style="display: none" class="position-relative">
-                <div class="mt-3 p-2 pt-3 rounded-2 border bg-light" id="assistant_response"></div>
+            <div id="response_block" class="position-relative d-none">
+                <div class="mt-3 p-2 pt-4 rounded-2 border bg-light">
+                    <div id="assistant_response"></div>
+                    <p class="spinner-border text-primary d-none" id="assistant_spinner" role="status">
+                        <span class="visually-hidden">Loading...</span>
+                    </p>
+                </div>
             </div>
         </div>
     </div>
diff --git a/explorer/tests/urls.py b/explorer/tests/urls.py
deleted file mode 100644
index 9fab3191..00000000
--- a/explorer/tests/urls.py
+++ /dev/null
@@ -1,14 +0,0 @@
-from django.contrib import admin
-from django.contrib.staticfiles.urls import staticfiles_urlpatterns
-from django.urls import path
-
-from explorer.urls import urlpatterns
-
-
-admin.autodiscover()
-
-urlpatterns += [
-    path("admin/", admin.site.urls),
-]
-
-urlpatterns += staticfiles_urlpatterns()
diff --git a/explorer/tracker.py b/explorer/tracker.py
index 3cfc46fa..c7d591ae 100644
--- a/explorer/tracker.py
+++ b/explorer/tracker.py
@@ -14,6 +14,8 @@
 from django.db.migrations.recorder import MigrationRecorder
 from django.conf import settings
 
+import explorer
+
 logger = logging.getLogger(__name__)
 
 
@@ -84,7 +86,7 @@ def _send(data):
                       data=data,
                       headers={"Content-Type": "application/json"})
     except Exception as e:
-        logger.exception("Failed to send stats: %s" % e)
+        logger.warning("Failed to send stats: %s" % e)
 
 
 def gather_summary_stats():
@@ -117,6 +119,7 @@ def gather_summary_stats():
             "unsafe_rendering": app_settings.UNSAFE_RENDERING,
             "transform_count": len(app_settings.EXPLORER_TRANSFORMS),
             "assistant_enabled": app_settings.EXPLORER_AI_API_KEY is not None,
+            "version": explorer.get_version()
         }
     except Exception as e:
         return {"error": "error gathering stats: %s" % e}
diff --git a/test_project/settings.py b/test_project/settings.py
index f506c155..78088cdcc 100644
--- a/test_project/settings.py
+++ b/test_project/settings.py
@@ -42,7 +42,7 @@
 }
 EXPLORER_DEFAULT_CONNECTION = "default"
 
-ROOT_URLCONF = "explorer.tests.urls"
+ROOT_URLCONF = "test_project.urls"
 
 TEMPLATES = [
     {
diff --git a/test_project/urls.py b/test_project/urls.py
new file mode 100644
index 00000000..677d8d26
--- /dev/null
+++ b/test_project/urls.py
@@ -0,0 +1,17 @@
+from django.contrib import admin
+from django.contrib.staticfiles.urls import staticfiles_urlpatterns
+from django.urls import path, include
+
+# Installing to /explorer/ better mimics likely production setups
+# Explorer is probably *not* running at the Django project root
+urlpatterns = [
+    path("explorer/", include("explorer.urls"))
+]
+
+admin.autodiscover()
+
+urlpatterns += [
+    path("admin/", admin.site.urls),
+]
+
+urlpatterns += staticfiles_urlpatterns()