diff --git a/datasette/templates/execute_write.html b/datasette/templates/execute_write.html index 6b626f8d..ee251111 100644 --- a/datasette/templates/execute_write.html +++ b/datasette/templates/execute_write.html @@ -40,6 +40,26 @@ border-radius: 0.25rem; min-width: 13rem; } +.execute-write-submit-row { + align-items: center; + display: flex; + flex-wrap: wrap; + gap: 0.45rem 0.75rem; +} +.execute-write-submit-row [hidden] { + display: none; +} +form.sql.core input[data-execute-write-submit]:disabled { + background: #d0d7de; + border-color: #b6c0cc; + color: #5f6975; + cursor: not-allowed; + opacity: 1; +} +.execute-write-disabled-reason { + color: #4f5b6d; + font-size: 0.85rem; +} {% include "_execute_write_analysis_styles.html" %} {% include "_sql_parameter_styles.html" %} @@ -119,9 +139,10 @@ {% endif %} -
- - {% if save_query_base_url %}Save this query{% endif %} +
+ + {{ execute_disabled_reason or "" }} + {% if save_query_url %}Save this query{% endif %}
@@ -143,25 +164,55 @@ window.addEventListener("DOMContentLoaded", () => { const submitButton = form ? form.querySelector("[data-execute-write-submit]") : null; - const saveQueryLink = form + const submitDisabledReason = form + ? form.querySelector("[data-execute-write-disabled-reason]") + : null; + const submitRow = form + ? form.querySelector(".execute-write-submit-row") + : null; + let saveQueryLink = form ? form.querySelector("[data-save-query-link]") : null; + function updateSubmitState(data) { + if (submitButton) { + submitButton.disabled = data.execute_disabled; + } + if (!submitDisabledReason) { + return; + } + const reason = data.execute_disabled_reason || ""; + submitDisabledReason.textContent = reason; + submitDisabledReason.hidden = !reason; + } + function updateSaveQueryLink(data) { - if (!saveQueryLink) { + if (!submitRow || !submitRow.dataset.saveQueryBaseUrl) { return; } const sql = window.editor ? window.editor.state.doc.toString() : executeWriteSqlInput.value; if (!sql.trim() || !data.ok || data.execute_disabled) { - saveQueryLink.hidden = true; + if (saveQueryLink) { + saveQueryLink.remove(); + saveQueryLink = null; + } return; } - const url = new URL(saveQueryLink.dataset.saveQueryBaseUrl, window.location.href); + if (!saveQueryLink) { + saveQueryLink = document.createElement("a"); + saveQueryLink.className = "save-query"; + saveQueryLink.setAttribute("data-save-query-link", ""); + saveQueryLink.textContent = "Save this query"; + submitRow.appendChild(saveQueryLink); + } + const url = new URL( + submitRow.dataset.saveQueryBaseUrl, + window.location.href + ); url.searchParams.set("sql", sql); saveQueryLink.href = url.pathname + url.search + url.hash; - saveQueryLink.hidden = false; } window.datasetteSqlParameters.setupSqlParameterRefresh({ @@ -170,9 +221,7 @@ window.addEventListener("DOMContentLoaded", () => { allowExpand: true, onData(data) { window.datasetteSqlAnalysis.renderAnalysis(analysisSection, data); - if (submitButton) { - submitButton.disabled = data.execute_disabled; - } + updateSubmitState(data); updateSaveQueryLink(data); }, onError(error) { @@ -180,12 +229,11 @@ window.addEventListener("DOMContentLoaded", () => { analysis_error: error.message, analysis_rows: [], }); - if (submitButton) { - submitButton.disabled = true; - } - if (saveQueryLink) { - saveQueryLink.hidden = true; - } + updateSubmitState({ + execute_disabled: true, + execute_disabled_reason: error.message, + }); + updateSaveQueryLink({ ok: false, execute_disabled: true }); }, }); }); diff --git a/datasette/views/execute_write.py b/datasette/views/execute_write.py index 57c4d78e..7b693978 100644 --- a/datasette/views/execute_write.py +++ b/datasette/views/execute_write.py @@ -14,6 +14,7 @@ from .query_helpers import ( _coerce_execute_write_payload, _derived_query_parameters, _execute_write_analysis_data, + _execute_write_disabled_reason, _inserted_row_url, _json_or_form_payload, _prepare_execute_write, @@ -80,13 +81,12 @@ class ExecuteWriteView(BaseView): ) save_query_base_url = None save_query_url = None + execute_disabled_reason = _execute_write_disabled_reason( + sql, analysis_error, analysis_rows + ) if allow_save_query: save_query_base_url = self.ds.urls.database(db.name) + "/-/queries/store" - if ( - sql - and analysis_error is None - and not any(row["allowed"] is False for row in analysis_rows) - ): + if not execute_disabled_reason: save_query_url = save_query_base_url + "?" + urlencode({"sql": sql}) response = await self.render( @@ -103,11 +103,8 @@ class ExecuteWriteView(BaseView): "execution_message": execution_message, "execution_links": execution_links, "execution_ok": execution_ok, - "execute_disabled": bool( - (not sql) - or analysis_error - or any(row["allowed"] is False for row in analysis_rows) - ), + "execute_disabled": bool(execute_disabled_reason), + "execute_disabled_reason": execute_disabled_reason, "table_columns": table_columns, "write_template_tables": write_template_tables, "save_query_url": save_query_url, diff --git a/datasette/views/query_helpers.py b/datasette/views/query_helpers.py index 712832e8..f30a30bc 100644 --- a/datasette/views/query_helpers.py +++ b/datasette/views/query_helpers.py @@ -268,6 +268,16 @@ async def _analysis_rows_with_permissions( return rows +def _execute_write_disabled_reason(sql, analysis_error, analysis_rows): + if not (sql and sql.strip()): + return "Enter writable SQL before executing." + if analysis_error: + return analysis_error + if any(row.get("allowed") is False for row in analysis_rows): + return "You do not have permission for every operation listed above." + return None + + def _coerce_execute_write_payload(data, is_json): if not isinstance(data, dict): raise QueryValidationError("JSON must be a dictionary") @@ -358,16 +368,16 @@ async def _execute_write_analysis_data(datasette, db, sql, actor): ) except (QueryValidationError, sqlite3.DatabaseError) as ex: analysis_error = getattr(ex, "message", str(ex)) + execute_disabled_reason = _execute_write_disabled_reason( + sql, analysis_error, analysis_rows + ) return { "ok": analysis_error is None, "parameters": parameter_names, "analysis_error": analysis_error, "analysis_rows": analysis_rows, - "execute_disabled": bool( - (not sql) - or analysis_error - or any(row["allowed"] is False for row in analysis_rows) - ), + "execute_disabled": bool(execute_disabled_reason), + "execute_disabled_reason": execute_disabled_reason, } diff --git a/tests/test_queries.py b/tests/test_queries.py index 2aa5142b..87ecacde 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -1374,6 +1374,10 @@ async def test_execute_write_get_prepopulates_without_executing(): assert 'addEventListener("paste"' in response.text assert "setupSqlParameterRefresh" in response.text assert "datasetteSqlAnalysis.renderAnalysis" in response.text + assert "input[data-execute-write-submit]:disabled" in response.text + assert ( + 'data-execute-write-disabled-reason aria-live="polite" hidden' in response.text + ) assert '| Required permission | ' in response.text assert "insert | " in response.text
@@ -1390,7 +1394,9 @@ async def test_execute_write_get_prepopulates_without_executing():
)
assert '' in empty_response.text
assert 'executeWriteSqlInput.value = "\\n\\n\\n";' in empty_response.text
- assert "hidden>Save this query" in empty_response.text
+ assert "Enter writable SQL before executing." in empty_response.text
+ assert 'data-save-query-base-url="/data/-/queries/store"' in empty_response.text
+ assert 'Save this query" in read_only_response.text
+ assert (
+ ''
+ ) in read_only_response.text
+ assert 'data-save-query-base-url="/data/-/queries/store"' in read_only_response.text
+ assert ''
+ ) in response.text
+ assert (
+ ''
+ "You do not have permission for every operation listed above."
+ ) in response.text
+ assert 'no' in response.text
+ assert 'data-save-query-base-url="/data/-/queries/store"' in response.text
+ assert '
|---|