diff --git a/datasette/app.py b/datasette/app.py index 40877802..bdbf9389 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1288,16 +1288,122 @@ class Datasette: ) return self._query_row_to_dict(rows.first()) - async def get_queries(self, database): - rows = await self.get_internal_database().execute( - """ - SELECT * FROM queries - WHERE database_name = ? - ORDER BY name - """, - [database], + async def list_queries( + self, + database, + *, + actor=None, + limit=50, + cursor=None, + q=None, + is_write=None, + is_published=None, + source=None, + owner_id=None, + include_private=False, + ): + limit = min(max(1, int(limit)), 1000) + allowed_sql, allowed_params = await self.allowed_resources_sql( + action="view-query", + actor=actor, + parent=database, + include_is_private=include_private, ) - return {row["name"]: self._query_row_to_dict(row) for row in rows} + params = dict(allowed_params) + params.update({"query_database": database, "limit": limit + 1}) + sort_key_sql = "lower(coalesce(nullif(q.title, ''), q.name))" + where_clauses = ["q.database_name = :query_database"] + + if cursor: + try: + components = urlsafe_components(cursor) + except ValueError: + components = [] + if len(components) == 2: + where_clauses.append(""" + ( + {sort_key_sql} > :cursor_sort_key + OR ( + {sort_key_sql} = :cursor_sort_key + AND q.name > :cursor_name + ) + ) + """.format(sort_key_sql=sort_key_sql)) + params["cursor_sort_key"] = components[0] + params["cursor_name"] = components[1] + + if q: + where_clauses.append(""" + ( + q.name LIKE :query_search + OR q.title LIKE :query_search + OR q.description LIKE :query_search + OR q.sql LIKE :query_search + ) + """) + params["query_search"] = "%{}%".format(q) + if is_write is not None: + where_clauses.append("q.is_write = :query_is_write") + params["query_is_write"] = int(bool(is_write)) + if is_published is not None: + where_clauses.append("q.is_published = :query_is_published") + params["query_is_published"] = int(bool(is_published)) + if source is not None: + where_clauses.append("q.source = :query_source") + params["query_source"] = source + if owner_id is not None: + where_clauses.append("q.owner_id = :query_owner_id") + params["query_owner_id"] = owner_id + + private_select = ", allowed.is_private AS private" if include_private else "" + rows = list( + ( + await self.get_internal_database().execute( + """ + SELECT q.*, {sort_key_sql} AS sort_key{private_select} + FROM queries q + JOIN ( + {allowed_sql} + ) allowed + ON allowed.parent = q.database_name + AND allowed.child = q.name + WHERE {where} + ORDER BY sort_key, q.name + LIMIT :limit + """.format( + allowed_sql=allowed_sql, + private_select=private_select, + sort_key_sql=sort_key_sql, + where=" AND ".join(where_clauses), + ), + params, + ) + ).rows + ) + has_more = len(rows) > limit + if has_more: + rows = rows[:limit] + + queries = [] + for row in rows: + query = self._query_row_to_dict(row) + if include_private: + query["private"] = bool(row["private"]) + queries.append(query) + + next_token = None + if has_more and rows: + last_row = rows[-1] + next_token = "{},{}".format( + tilde_encode(last_row["sort_key"]), + tilde_encode(last_row["name"]), + ) + return { + "queries": queries, + "next": next_token, + "has_more": has_more, + "limit": limit, + } async def ensure_query_write_permissions( self, database, sql, *, actor=None, params=None, analysis=None @@ -1564,7 +1670,8 @@ class Datasette: return self.static_hash("app.css") async def get_canned_queries(self, database_name, actor): - return await self.get_queries(database_name) + page = await self.list_queries(database_name, actor=actor, limit=1000) + return {query["name"]: query for query in page["queries"]} async def get_canned_query(self, database_name, query_name, actor): return await self.get_query(database_name, query_name) @@ -2591,7 +2698,7 @@ class Datasette: add_route(TableCreateView.as_view(self), r"/(?P[^\/\.]+)/-/create$") add_route( QueryListView.as_view(self), - r"/(?P[^\/\.]+)/-/queries$", + r"/(?P[^\/\.]+)/-/queries(\.(?Pjson))?$", ) add_route( QueryCreateView.as_view(self), diff --git a/datasette/templates/database.html b/datasette/templates/database.html index 42b4ca0b..a39d6ad7 100644 --- a/datasette/templates/database.html +++ b/datasette/templates/database.html @@ -53,6 +53,9 @@
  • {{ query.title or query.name }}{% if query.private %} 🔒{% endif %}
    • {% endfor %} + {% if queries_more %} +

    View all queries

    + {% endif %} {% endif %} {% if tables %} diff --git a/datasette/templates/query_list.html b/datasette/templates/query_list.html new file mode 100644 index 00000000..ef5da0d5 --- /dev/null +++ b/datasette/templates/query_list.html @@ -0,0 +1,55 @@ +{% extends "base.html" %} + +{% block title %}{{ database }}: queries{% endblock %} + +{% block body_class %}query-list db-{{ database|to_css_class }}{% endblock %} + +{% block crumbs %} +{{ crumbs.nav(request=request, database=database) }} +{% endblock %} + +{% block content %} + +

    Queries

    + +
    +

    + + + +

    +

    + + + + +

    +
    + +{% if queries %} +
      + {% for query in queries %} +
    • + {{ query.title or query.name }}{% if query.private %} 🔒{% endif %} + {% if query.is_write %}Writable{% endif %} + {% if query.is_published %}Published{% endif %} +
      • + {% endfor %} +
    +{% else %} +

    No queries found.

    +{% endif %} + +{% if next_url %} +

    Next page

    +{% endif %} + +{% endblock %} diff --git a/datasette/views/database.py b/datasette/views/database.py index ed38189b..edbc315e 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -92,24 +92,14 @@ class DatabaseView(View): tables = await get_tables(datasette, request, db, allowed_dict) - # Get allowed queries using the new permission system - allowed_query_page = await datasette.allowed_resources( - "view-query", - request.actor, - parent=database, - include_is_private=True, - limit=1000, + queries_page = await datasette.list_queries( + database, + actor=request.actor, + limit=20, + include_private=True, ) - - # Build canned_queries list by looking up each allowed query - all_queries = await datasette.get_canned_queries(database, request.actor) - canned_queries = [] - for query_resource in allowed_query_page.resources: - query_name = query_resource.child - if query_name in all_queries: - canned_queries.append( - dict(all_queries[query_name], private=query_resource.private) - ) + canned_queries = queries_page["queries"] + queries_more = queries_page["has_more"] async def database_actions(): links = [] @@ -141,6 +131,7 @@ class DatabaseView(View): "hidden_count": len([t for t in tables if t["hidden"]]), "views": sql_views, "queries": canned_queries, + "queries_more": queries_more, "allow_execute_sql": allow_execute_sql, "table_columns": ( await _table_columns(datasette, database) if allow_execute_sql else {} @@ -174,6 +165,7 @@ class DatabaseView(View): hidden_count=len([t for t in tables if t["hidden"]]), views=sql_views, queries=canned_queries, + queries_more=queries_more, allow_execute_sql=allow_execute_sql, table_columns=( await _table_columns(datasette, database) @@ -222,6 +214,9 @@ class DatabaseContext(Context): hidden_count: int = field(metadata={"help": "Count of hidden tables"}) views: list = field(metadata={"help": "List of view objects in the database"}) queries: list = field(metadata={"help": "List of canned query objects"}) + queries_more: bool = field( + metadata={"help": "Boolean indicating if more saved queries are available"} + ) allow_execute_sql: bool = field( metadata={"help": "Boolean indicating if custom SQL can be executed"} ) @@ -474,6 +469,31 @@ def _as_bool(value): return bool(value) +def _as_optional_bool(value, name): + if value is None or value == "": + return None + if isinstance(value, bool): + return value + if isinstance(value, int): + return bool(value) + if isinstance(value, str): + lowered = value.lower() + if lowered in {"1", "true", "t", "yes", "on"}: + return True + if lowered in {"0", "false", "f", "no", "off"}: + return False + raise QueryValidationError("{} must be 0 or 1".format(name)) + + +def _query_list_limit(value): + if value in (None, ""): + return 50 + try: + return min(max(1, int(value)), 1000) + except ValueError as ex: + raise QueryValidationError("_size must be an integer") from ex + + def _derived_query_parameters(sql): parameters = [] seen = set() @@ -949,19 +969,66 @@ class QueryListView(BaseView): async def get(self, request): db = await self.ds.resolve_database(request) - page = await self.ds.allowed_resources( - "view-query", - request.actor, - parent=db.name, - limit=1000, + format_ = request.url_vars.get("format") or "html" + try: + limit = _query_list_limit(request.args.get("_size")) + is_write = _as_optional_bool(request.args.get("is_write"), "is_write") + is_published = _as_optional_bool( + request.args.get("is_published"), "is_published" + ) + except QueryValidationError as ex: + return _error([ex.message], ex.status) + + page = await self.ds.list_queries( + db.name, + actor=request.actor, + limit=limit, + cursor=request.args.get("_next"), + q=request.args.get("q") or None, + is_write=is_write, + is_published=is_published, + source=request.args.get("source") or None, + owner_id=request.args.get("owner_id") or None, + include_private=True, + ) + next_url = None + if page["next"]: + pairs = [ + (key, value) + for key, value in parse_qsl( + request.query_string, keep_blank_values=True + ) + if key != "_next" + ] + pairs.append(("_next", page["next"])) + next_url = "{}?{}".format( + self.ds.urls.database(db.name) + "/-/queries", + urlencode(pairs), + ) + + data = { + "ok": True, + "database": db.name, + "queries": page["queries"], + "next": page["next"], + "next_url": next_url, + "has_more": page["has_more"], + "limit": page["limit"], + "filters": { + "q": request.args.get("q") or "", + "is_write": request.args.get("is_write") or "", + "is_published": request.args.get("is_published") or "", + "source": request.args.get("source") or "", + "owner_id": request.args.get("owner_id") or "", + }, + } + if format_ == "json": + return Response.json(data) + return await self.render( + ["query_list.html"], + request, + data, ) - all_queries = await self.ds.get_queries(db.name) - queries = [ - all_queries[resource.child] - for resource in page.resources - if resource.child in all_queries - ] - return Response.json({"ok": True, "database": db.name, "queries": queries}) class QueryCreateView(BaseView): diff --git a/docs/json_api.rst b/docs/json_api.rst index e4c9e86e..ece430c2 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -510,7 +510,7 @@ Datasette provides a write API for JSON data. This is a POST-only API that requi Listing saved queries ~~~~~~~~~~~~~~~~~~~~~ -``GET //-/queries`` returns saved query definitions the actor can view. +``GET //-/queries.json`` returns saved query definitions the actor can view. Use ``?_size=50`` to set the page size and ``?_next=...`` with the cursor returned by the previous page to fetch the next page. .. _QueryCreateView: diff --git a/queries-plan.md b/queries-plan.md index a58ace70..671fc29c 100644 --- a/queries-plan.md +++ b/queries-plan.md @@ -210,7 +210,7 @@ JSON endpoints should follow Datasette's existing write API style: use `POST` pl Endpoints: -- `GET /{database}/-/queries` lists query definitions the actor can view or manage, probably paginated. +- `GET /{database}/-/queries` shows a searchable HTML query browser. `GET /{database}/-/queries.json` returns query definitions the actor can view, using cursor pagination with `_next` and `_size`. - `POST /{database}/-/queries/-/insert` creates a query. - `GET /{database}/{query}/-/definition` returns one query definition without executing it. - `POST /{database}/{query}/-/update` updates one query. @@ -353,9 +353,21 @@ await datasette.update_query( await datasette.remove_query(database, name, source=None) await datasette.get_query(database, name) -await datasette.get_queries(database) +await datasette.list_queries( + database, + actor=None, + limit=50, + cursor=None, + q=None, + is_write=None, + is_published=None, + source=None, + owner_id=None, +) ``` +`list_queries()` should return a bounded page shaped like `{"queries": [...], "next": "...", "has_more": true, "limit": 50}`. The `next` value is an opaque cursor token, not an offset. + `update_query()` should use an internal sentinel default such as `UNCHANGED = object()` so callers can distinguish "leave this column alone" from "set this column to `NULL`": ```python @@ -380,6 +392,8 @@ The save form should call `POST /{database}/-/queries/-/insert` and default to ` If the actor also has `publish-query`, include a publish control. The UI copy should make it clear that publishing allows people without arbitrary SQL permission to run this query. +On `/{database}`, show a preview of the first 20 visible queries using `list_queries(..., limit=20)`. If the page has `has_more`, show a link to `/{database}/-/queries` rather than rendering hundreds or thousands of query links inline. The full `/{database}/-/queries` page provides search, filters, and cursor pagination. + ## Dedicated create query UI Add `/{database}/-/queries/-/create` for the fuller query authoring flow, including writable queries. diff --git a/tests/test_queries.py b/tests/test_queries.py index df4131b9..dd906faf 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -7,6 +7,20 @@ from datasette.resources import DatabaseResource, QueryResource from datasette.utils.asgi import Forbidden +async def add_numbered_queries(ds, database, count): + for i in range(1, count + 1): + await ds.add_query( + database, + "demo_query_{:02d}".format(i), + "select {} as query_number".format(i), + title="Demo query {:02d}".format(i), + description="Seeded demo query number {:02d}".format(i), + is_published=True, + source="user", + owner_id="root", + ) + + @pytest.mark.asyncio async def test_queries_internal_table_schema(): ds = Datasette(memory=True) @@ -96,11 +110,15 @@ async def test_add_get_and_remove_query(): "on_error_redirect": None, } - assert await ds.get_queries("data") == {"top_customers": query} + queries_page = await ds.list_queries("data", actor=None) + assert queries_page["queries"] == [query] + assert queries_page["next"] is None await ds.remove_query("data", "top_customers") assert await ds.get_query("data", "top_customers") is None - assert await ds.get_queries("data") == {} + queries_page = await ds.list_queries("data", actor=None) + assert queries_page["queries"] == [] + assert queries_page["next"] is None @pytest.mark.asyncio @@ -238,6 +256,24 @@ async def test_unpublished_query_requires_execute_sql_but_published_does_not(): ) +@pytest.mark.asyncio +async def test_database_page_query_preview_is_limited(): + ds = Datasette(memory=True) + ds.add_memory_database("query_preview", name="data") + await ds.invoke_startup() + await add_numbered_queries(ds, "data", 25) + + html_response = await ds.client.get("/data") + json_response = await ds.client.get("/data.json") + + assert html_response.status_code == 200 + assert "Demo query 20" in html_response.text + assert "Demo query 21" not in html_response.text + assert 'href="/data/-/queries"' in html_response.text + assert len(json_response.json()["queries"]) == 20 + assert json_response.json()["queries_more"] is True + + @pytest.mark.asyncio async def test_query_actions_are_registered(): ds = Datasette() @@ -347,21 +383,78 @@ async def test_query_list_and_definition_api(): ds.root_enabled = True ds.add_memory_database("query_list_api", name="data") await ds.invoke_startup() - await ds.add_query("data", "listed", "select 1", title="Listed", is_published=True) + await add_numbered_queries(ds, "data", 12) list_response = await ds.client.get( - "/data/-/queries", + "/data/-/queries.json?_size=5", + actor={"id": "root"}, + ) + next_response = await ds.client.get( + "/data/-/queries.json?_size=5&_next={}".format(list_response.json()["next"]), actor={"id": "root"}, ) definition_response = await ds.client.get( - "/data/listed/-/definition", + "/data/demo_query_01/-/definition", actor={"id": "root"}, ) assert list_response.status_code == 200 - assert list_response.json()["queries"][0]["name"] == "listed" + assert [query["name"] for query in list_response.json()["queries"]] == [ + "demo_query_01", + "demo_query_02", + "demo_query_03", + "demo_query_04", + "demo_query_05", + ] + assert list_response.json()["next"] + assert [query["name"] for query in next_response.json()["queries"]] == [ + "demo_query_06", + "demo_query_07", + "demo_query_08", + "demo_query_09", + "demo_query_10", + ] assert definition_response.status_code == 200 - assert definition_response.json()["query"]["title"] == "Listed" + assert definition_response.json()["query"]["title"] == "Demo query 01" + + +@pytest.mark.asyncio +async def test_query_list_search_filter_and_html(): + ds = Datasette(memory=True) + ds.root_enabled = True + ds.add_memory_database("query_list_html", name="data") + await ds.invoke_startup() + await add_numbered_queries(ds, "data", 3) + await ds.add_query( + "data", + "private_query", + "select 'private'", + title="Private query", + is_published=False, + source="user", + owner_id="root", + ) + + html_response = await ds.client.get( + "/data/-/queries?q=02", + actor={"id": "root"}, + ) + json_response = await ds.client.get( + "/data/-/queries.json?q=02", + actor={"id": "root"}, + ) + filtered_response = await ds.client.get( + "/data/-/queries.json?is_published=0", + actor={"id": "root"}, + ) + + assert html_response.status_code == 200 + assert "Demo query 02" in html_response.text + assert "Demo query 01" not in html_response.text + assert json_response.json()["queries"][0]["name"] == "demo_query_02" + assert [query["name"] for query in filtered_response.json()["queries"]] == [ + "private_query" + ] @pytest.mark.asyncio