Migrate view-query permission to SQL-based system, refs #2510

This change integrates canned queries with Datasette's new SQL-based
permissions system by making the following changes:

1. **Default canned_queries plugin hook**: Added a new hookimpl in
   default_permissions.py that returns canned queries from datasette
   configuration. This extracts config-reading logic into a plugin hook,
   allowing QueryResource to discover all queries.

2. **Async resources_sql()**: Converted Resource.resources_sql() from a
   synchronous class method returning a string to an async method that
   receives the datasette instance. This allows QueryResource to call
   plugin hooks and query the database.

3. **QueryResource implementation**: Implemented QueryResource.resources_sql()
   to gather all canned queries by:
   - Querying catalog_databases for all databases
   - Calling canned_queries hooks for each database with actor=None
   - Building a UNION ALL SQL query of all (database, query_name) pairs
   - Properly escaping single quotes in resource names

4. **Simplified get_canned_queries()**: Removed config-reading logic since
   it's now handled by the default plugin hook.

5. **Added view-query to default allow**: Added "view-query" to the
   default_allow_actions set so canned queries are accessible by default.

6. **Removed xfail markers**: Removed test xfail markers from:
   - tests/test_canned_queries.py (entire module)
   - tests/test_html.py (2 tests)
   - tests/test_permissions.py (1 test)
   - tests/test_plugins.py (1 test)

All canned query tests now pass with the new permission system.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

datasette/app.py

@@ -922,9 +922,7 @@ class Datasette:
         return self._app_css_hash
 
     async def get_canned_queries(self, database_name, actor):
-        queries = (
+        queries = {}
-            ((self.config or {}).get("databases") or {}).get(database_name) or {}
-        ).get("queries") or {}
         for more_queries in pm.hook.canned_queries(
             datasette=self,
             database=database_name,

datasette/default_permissions.py

@@ -50,6 +50,7 @@ async def permission_resources_sql(datasette, actor, action):
         "view-database",
         "view-database-download",
         "view-table",
+        "view-query",
         "execute-sql",
     }
     if action in default_allow_actions:
@@ -335,3 +336,12 @@ def skip_csrf(scope):
         headers = scope.get("headers") or {}
         if dict(headers).get(b"content-type") == b"application/json":
             return True
+
+
+@hookimpl
+def canned_queries(datasette, database, actor):
+    """Return canned queries from datasette configuration."""
+    queries = (
+        ((datasette.config or {}).get("databases") or {}).get(database) or {}
+    ).get("queries") or {}
+    return queries

datasette/resources.py

@@ -13,7 +13,7 @@ class InstanceResource(Resource):
         super().__init__(parent=None, child=None)
 
     @classmethod
-    def resources_sql(cls) -> str:
+    async def resources_sql(cls, datasette) -> str:
         return "SELECT NULL AS parent, NULL AS child"
 
 
@@ -27,7 +27,7 @@ class DatabaseResource(Resource):
         super().__init__(parent=database, child=None)
 
     @classmethod
-    def resources_sql(cls) -> str:
+    async def resources_sql(cls, datasette) -> str:
         return """
             SELECT database_name AS parent, NULL AS child
             FROM catalog_databases
@@ -44,7 +44,7 @@ class TableResource(Resource):
         super().__init__(parent=database, child=table)
 
     @classmethod
-    def resources_sql(cls) -> str:
+    async def resources_sql(cls, datasette) -> str:
         return """
             SELECT database_name AS parent, table_name AS child
             FROM catalog_tables
@@ -64,6 +64,41 @@ class QueryResource(Resource):
         super().__init__(parent=database, child=query)
 
     @classmethod
-    def resources_sql(cls) -> str:
+    async def resources_sql(cls, datasette) -> str:
-        # TODO: Need catalog for queries
+        from datasette.plugins import pm
-        return "SELECT NULL AS parent, NULL AS child WHERE 0"
+        from datasette.utils import await_me_maybe
+
+        # Get all databases from catalog
+        db = datasette.get_internal_database()
+        result = await db.execute("SELECT database_name FROM catalog_databases")
+        databases = [row[0] for row in result.rows]
+
+        # Gather all canned queries from all databases
+        query_pairs = []
+        for database_name in databases:
+            # Call the hook to get queries (including from config via default plugin)
+            for queries_result in pm.hook.canned_queries(
+                datasette=datasette,
+                database=database_name,
+                actor=None,  # Get ALL queries for resource enumeration
+            ):
+                queries = await await_me_maybe(queries_result)
+                if queries:
+                    for query_name in queries.keys():
+                        query_pairs.append((database_name, query_name))
+
+        # Build SQL
+        if not query_pairs:
+            return "SELECT NULL AS parent, NULL AS child WHERE 0"
+
+        # Generate UNION ALL query
+        selects = []
+        for db_name, query_name in query_pairs:
+            # Escape single quotes by doubling them
+            db_escaped = db_name.replace("'", "''")
+            query_escaped = query_name.replace("'", "''")
+            selects.append(
+                f"SELECT '{db_escaped}' AS parent, '{query_escaped}' AS child"
+            )
+
+        return " UNION ALL ".join(selects)

datasette/utils/actions_sql.py

@@ -177,7 +177,7 @@ async def _build_single_action_sql(
         raise ValueError(f"Unknown action: {action}")
 
     # Get base resources SQL from the resource class
-    base_resources_sql = action_obj.resource_class.resources_sql()
+    base_resources_sql = await action_obj.resource_class.resources_sql(datasette)
 
     # Get all permission rule fragments from plugins via the hook
     rule_results = pm.hook.permission_resources_sql(

tests/test_api.py

@@ -898,9 +898,6 @@ async def test_json_columns(ds_client, extra_args, expected):
     assert response.json() == expected
 
 
-@pytest.mark.xfail(
-    reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-)
 def test_config_cache_size(app_client_larger_cache_size):
     response = app_client_larger_cache_size.get("/fixtures/pragma_cache_size.json")
     assert response.json["rows"] == [{"cache_size": -2500}]

tests/test_canned_queries.py

@@ -4,11 +4,6 @@ import pytest
 import re
 from .fixtures import make_app_client, app_client
 
-# Mark entire module as xfail since view-query permission not yet migrated, refs #2534
-pytestmark = pytest.mark.xfail(
-    reason="view-query permission not yet migrated to new permission system, refs #2534"
-)
-
 
 @pytest.fixture
 def canned_write_client(tmpdir):

tests/test_html.py

@@ -135,9 +135,6 @@ def test_not_allowed_methods():
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not displayed due to view-query permission, refs #2510"
-)
 async def test_database_page(ds_client):
     response = await ds_client.get("/fixtures")
     soup = Soup(response.text, "html.parser")
@@ -263,10 +260,9 @@ def test_query_page_truncates():
             "/fixtures/simple_primary_key",
             ["table", "db-fixtures", "table-simple_primary_key"],
         ),
-        pytest.param(
+        (
             "/fixtures/neighborhood_search",
             ["query", "db-fixtures", "query-neighborhood_search"],
-            marks=pytest.mark.xfail(reason="Canned queries not accessible, refs #2510"),
         ),
         (
             "/fixtures/table~2Fwith~2Fslashes~2Ecsv",
@@ -599,9 +595,6 @@ async def test_404_content_type(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_canned_query_default_title(ds_client):
     response = await ds_client.get("/fixtures/magic_parameters")
     assert response.status_code == 200
@@ -610,9 +603,6 @@ async def test_canned_query_default_title(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_canned_query_with_custom_metadata(ds_client):
     response = await ds_client.get("/fixtures/neighborhood_search?text=town")
     assert response.status_code == 200
@@ -675,9 +665,6 @@ async def test_show_hide_sql_query(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_canned_query_with_hide_has_no_hidden_sql(ds_client):
     # For a canned query the show/hide should NOT have a hidden SQL field
     # https://github.com/simonw/datasette/issues/1411
@@ -689,9 +676,6 @@ async def test_canned_query_with_hide_has_no_hidden_sql(ds_client):
     ] == [(hidden["name"], hidden["value"]) for hidden in hiddens]
 
 
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 @pytest.mark.parametrize(
     "hide_sql,querystring,expected_hidden,expected_show_hide_link,expected_show_hide_text",
     (
@@ -941,9 +925,6 @@ def test_base_url_affects_metadata_extra_css_urls(app_client_base_url_prefix):
         ("/fixtures/magic_parameters", None),
     ],
 )
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_edit_sql_link_on_canned_queries(ds_client, path, expected):
     response = await ds_client.get(path)
     assert response.status_code == 200
@@ -959,9 +940,6 @@ async def test_edit_sql_link_on_canned_queries(ds_client, path, expected):
     [
         pytest.param(
             True,
-            marks=pytest.mark.xfail(
-                reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-            ),
         ),
         False,
     ],
@@ -1057,10 +1035,9 @@ async def test_trace_correctly_escaped(ds_client):
             "http://localhost/fixtures/-/query.json?sql=select+*+from+facetable",
         ),
         # Canned query page
-        pytest.param(
+        (
             "/fixtures/neighborhood_search?text=town",
             "http://localhost/fixtures/neighborhood_search.json?text=town",
-            marks=pytest.mark.xfail(reason="Canned queries not accessible, refs #2510"),
         ),
         # /-/ pages
         (
@@ -1180,7 +1157,7 @@ async def test_database_color(ds_client):
         "/fixtures",
         "/fixtures/facetable",
         "/fixtures/paginated_view",
-        # "/fixtures/pragma_cache_size",  # Canned query - skipped due to view-query not migrated, refs #2510
+        "/fixtures/pragma_cache_size",
     ):
         response = await ds_client.get(path)
         assert any(

tests/test_permissions.py

@@ -234,7 +234,6 @@ def test_table_list_respects_view_table():
             assert html_fragment in auth_response.text
 
 
-@pytest.mark.xfail(reason="view-query not yet migrated to new permission system")
 @pytest.mark.parametrize(
     "allow,expected_anon,expected_auth",
     [
@@ -365,9 +364,6 @@ def test_query_list_respects_view_query():
                 ("view-database", "fixtures"),
                 ("view-query", ("fixtures", "neighborhood_search")),
             ],
-            marks=pytest.mark.xfail(
-                reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-            ),
         ),
     ],
 )
@@ -593,9 +589,6 @@ def test_permissions_cascade(cascade_app_client, path, permissions, expected_sta
         cascade_app_client.ds.config = previous_config
 
 
-@pytest.mark.xfail(
-    reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-)
 def test_padlocks_on_database_page(cascade_app_client):
     config = {
         "databases": {

tests/test_plugins.py

@@ -499,9 +499,6 @@ async def test_hook_register_output_renderer_all_parameters(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-)
 async def test_hook_register_output_renderer_custom_status_code(ds_client):
     response = await ds_client.get(
         "/fixtures/pragma_cache_size.testall?status_code=202"
@@ -510,9 +507,6 @@ async def test_hook_register_output_renderer_custom_status_code(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-)
 async def test_hook_register_output_renderer_custom_content_type(ds_client):
     response = await ds_client.get(
         "/fixtures/pragma_cache_size.testall?content_type=text/blah"
@@ -521,9 +515,6 @@ async def test_hook_register_output_renderer_custom_content_type(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-)
 async def test_hook_register_output_renderer_custom_headers(ds_client):
     response = await ds_client.get(
         "/fixtures/pragma_cache_size.testall?header=x-wow:1&header=x-gosh:2"
@@ -854,9 +845,6 @@ async def test_hook_startup(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_hook_canned_queries(ds_client):
     queries = (await ds_client.get("/fixtures.json")).json()["queries"]
     queries_by_name = {q["name"]: q for q in queries}
@@ -873,34 +861,24 @@ async def test_hook_canned_queries(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_hook_canned_queries_non_async(ds_client):
     response = await ds_client.get("/fixtures/from_hook.json?_shape=array")
     assert [{"1": 1, "actor_id": "null"}] == response.json()
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_hook_canned_queries_async(ds_client):
     response = await ds_client.get("/fixtures/from_async_hook.json?_shape=array")
     assert [{"2": 2}] == response.json()
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_hook_canned_queries_actor(ds_client):
     assert (
         await ds_client.get("/fixtures/from_hook.json?_bot=1&_shape=array")
     ).json() == [{"1": 1, "actor_id": "bot"}]
 
 
-@pytest.mark.xfail(reason="Magic parameters used with canned queries, refs #2510")
 def test_hook_register_magic_parameters(restore_working_directory):
     with make_app_client(
         extra_databases={"data.db": "create table logs (line text)"},
@@ -1048,9 +1026,6 @@ def get_actions_links(html):
         pytest.param(
             "/fixtures/pragma_cache_size",
             "/fixtures/-/query?sql=explain+PRAGMA+cache_size%3B",
-            marks=pytest.mark.xfail(
-                reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-            ),
         ),
         # Don't attempt to explain an explain
         ("/fixtures/-/query?sql=explain+select+1", None),
@@ -1558,9 +1533,6 @@ async def test_hook_top_query(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not yet migrated to new permission system, refs #2510"
-)
 async def test_hook_top_canned_query(ds_client):
     try:
         pm.register(SlotPlugin(), name="SlotPlugin")

tests/test_table_api.py

@@ -1147,9 +1147,6 @@ async def test_infinity_returned_as_invalid_json_if_requested(ds_client):
 
 
 @pytest.mark.asyncio
-@pytest.mark.xfail(
-    reason="Canned queries not accessible due to view-query permission not migrated, refs #2510"
-)
 async def test_custom_query_with_unicode_characters(ds_client):
     # /fixtures/𝐜𝐢𝐭𝐢𝐞𝐬.json
     response = await ds_client.get(