Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability for a canned query to write to the database #698

Closed
simonw opened this issue Mar 16, 2020 · 26 comments
Closed

Ability for a canned query to write to the database #698

simonw opened this issue Mar 16, 2020 · 26 comments

Comments

@simonw
Copy link
Owner

simonw commented Mar 16, 2020

Canned queries are currently read-only: https://datasette.readthedocs.io/en/0.38/sql_queries.html#canned-queries

Add a "write": true option to their definition in metadata.json which turns them into queries that are submitted via POST and send their queries to the write queue.

Then they can be used as a really quick way to define a writable interface and JSON API!

@simonw simonw added the feature label Mar 16, 2020
@simonw
Copy link
Owner Author

simonw commented Mar 16, 2020

{
    "databases": {
       "my-database": {
           "queries": {
               "add_twitter_handle": {
                   "sql": "insert into twitter_handles (username) values (:username)",
                   "write": true
               }
           }
       }
    }
}

@simonw
Copy link
Owner Author

simonw commented Mar 16, 2020

By default this will extract the :params using the existing regular expression - which can occasionally break if there is a rogue : in the rest of the query.

To address this: allow an extra optional "params": ["username"] field in metadata which, if available, is used instead of the regular expression extraction.

@simonw
Copy link
Owner Author

simonw commented Mar 16, 2020

This is going to need CSRF protection - see https://github.com/simonw/asgi-csrf

@simonw
Copy link
Owner Author

simonw commented Mar 16, 2020

Also relevant: this will benefit from an authentication/permissions layer: #699

@simonw
Copy link
Owner Author

simonw commented Mar 16, 2020

This is also going to need me to handle POST form submissions which means I need to be able to parse the form body. I guess that will go in datasette/utils/asgi.py.

@simonw
Copy link
Owner Author

simonw commented Mar 16, 2020

@simonw
Copy link
Owner Author

simonw commented Mar 16, 2020

It would be useful if this feature supported different types of input - at the most basic level <input type="text"> v.s. <textarea>, but numbers, <select> etc would be useful too. This can be added later as additional metadata.json settings.

@simonw
Copy link
Owner Author

simonw commented Mar 17, 2020

This is going to require a pretty full rewrite of the existing canned query logic, which currently shares a codepath with the code that lets you submit a SQL query:

async def custom_sql(
self,
request,
database,
hash,
sql,
editable=True,
canned_query=None,
metadata=None,
_size=None,
):
params = request.raw_args
if "sql" in params:
params.pop("sql")
if "_shape" in params:
params.pop("_shape")
# Extract any :named parameters
named_parameters = self.re_named_parameter.findall(sql)
named_parameter_values = {
named_parameter: params.get(named_parameter) or ""
for named_parameter in named_parameters
}
# Set to blank string if missing from params
for named_parameter in named_parameters:
if named_parameter not in params:
params[named_parameter] = ""
extra_args = {}
if params.get("_timelimit"):
extra_args["custom_time_limit"] = int(params["_timelimit"])
if _size:
extra_args["page_size"] = _size
results = await self.ds.execute(
database, sql, params, truncate=True, **extra_args
)
columns = [r[0] for r in results.description]
templates = ["query-{}.html".format(to_css_class(database)), "query.html"]
if canned_query:
templates.insert(
0,
"query-{}-{}.html".format(
to_css_class(database), to_css_class(canned_query)
),
)
async def extra_template():
display_rows = []
for row in results.rows:
display_row = []
for column, value in zip(results.columns, row):
display_value = value
# Let the plugins have a go
# pylint: disable=no-member
plugin_value = pm.hook.render_cell(
value=value,
column=column,
table=None,
database=database,
datasette=self.ds,
)
if plugin_value is not None:
display_value = plugin_value
else:
if value in ("", None):
display_value = jinja2.Markup("&nbsp;")
elif is_url(str(display_value).strip()):
display_value = jinja2.Markup(
'<a href="{url}">{url}</a>'.format(
url=jinja2.escape(value.strip())
)
)
display_row.append(display_value)
display_rows.append(display_row)
return {
"display_rows": display_rows,
"custom_sql": True,
"named_parameter_values": named_parameter_values,
"editable": editable,
"canned_query": canned_query,
"metadata": metadata,
"config": self.ds.config_dict(),
"request": request,
"path_with_added_args": path_with_added_args,
"path_with_removed_args": path_with_removed_args,
"hide_sql": "_hide_sql" in params,
}
return (
{
"database": database,
"rows": results.rows,
"truncated": results.truncated,
"columns": columns,
"query": {"sql": sql, "params": params},
},
extra_template,
templates,
)

@simonw
Copy link
Owner Author

simonw commented Mar 22, 2020

This feature should include the ability to set a custom redirect URL for after the query has been executed - that way it can be used to build things like "delete this row" which redirect back to the correct table.

@simonw simonw pinned this issue Mar 24, 2020
@simonw simonw modified the milestone: Datasette 0.39 Mar 24, 2020
simonw added a commit that referenced this issue Mar 26, 2020
@simonw
Copy link
Owner Author

simonw commented Apr 2, 2020

YAML for metadata is relevant to this: #713

@simonw
Copy link
Owner Author

simonw commented Apr 2, 2020

I really want the option to use a <textarea> for a specific value.

Idea: metadata syntax like this:

{
    "databases": {
       "my-database": {
           "queries": {
               "add_twitter_handle": {
                   "sql": "insert into twitter_handles (username) values (:username)",
                   "write": true,
                   "params": {
                       "username": {
                            "widget": "textarea"
                       }
                   }
               }
           }
       }
    }
}

I can ship with some default widgets and provide a plugin hook for registering extra widgets.

This opens up some really exciting possibilities for things like map widgets that let you draw polygons.

@simonw
Copy link
Owner Author

simonw commented Apr 29, 2020

What should happen when a query has been successfully executed?

That depends on the query. Some queries may wish to redirect to another page. Other queries might want to show a custom message.

There should at least be a default message saying the query has been executed.

@simonw
Copy link
Owner Author

simonw commented Apr 29, 2020

Concept for displaying a success message:

fixtures__compound_three_primary_keys__1_001_rows

CSS:
.success {
    padding: 1em;
    border: 1px solid green;
    background-color: #c7fbc7;
}

@simonw
Copy link
Owner Author

simonw commented Apr 29, 2020

Need to figure out what the .json mode for this looks like - and if there's a .csv mode (I think not).

simonw added a commit that referenced this issue Jun 3, 2020
@simonw
Copy link
Owner Author

simonw commented Jun 3, 2020

Started a fresh pull request for this in #796 - the one in #703 got a bit untidy.

@simonw
Copy link
Owner Author

simonw commented Jun 3, 2020

Some extra thoughts now that this is mostly working:

  • "Edit this row" is such an obvious use-case. Could I automatically support row editing where every column except the primary key can be updated?
  • It would be useful to be able to link to a query in a way that pre-populates various form fields. The "edit" interface could then be a link that pre-populates the form with all of the existing values.
  • Can the redirect URL be configured to include values from the form submission? So you could e.g. add a blog post with a unique slug and then redirect to that URL?

@simonw
Copy link
Owner Author

simonw commented Jun 3, 2020

One challenge with this feature is that it confuses the messaging about what Datasette does somewhat.

Prior to shipping this, Datasette's core value proposition is as a way to publish read-only data.

That changed a little in 0.37 in February when plugins gained the supported ability to execute writes, but there was no way of doing that without a plugin.

With this feature, Datasette becomes a read-write database solution.

I should update the documentation to help explain this. Essentially the message is that Datasette can be used in one of two "modes" - it can be used just for sharing/publishing data, or you can use it to collect and manage data, most likely still in collaboration with plugins for things like authentication.

simonw added a commit that referenced this issue Jun 3, 2020
Message also now shows number of affected rows after executing query.
simonw added a commit that referenced this issue Jun 3, 2020
simonw added a commit that referenced this issue Jun 3, 2020
Message also now shows number of affected rows after executing query.
simonw added a commit that referenced this issue Jun 3, 2020
Refs #698. First working version of this feature.

* request.post_vars() no longer discards empty values
@simonw
Copy link
Owner Author

simonw commented Jun 3, 2020

Landed the work so far from #796! Here's the documentation: https://datasette.readthedocs.io/en/latest/sql_queries.html#writable-canned-queries

@simonw simonw unpinned this issue Jun 4, 2020
@simonw simonw pinned this issue Jun 4, 2020
@simonw
Copy link
Owner Author

simonw commented Jun 5, 2020

CSRF is done. Last step: figure out a smart way to integrate this with permissions and authentication.

@simonw
Copy link
Owner Author

simonw commented Jun 5, 2020

Idea: default is anyone can execute a query.

Or you can specify the following:

{
    "databases": {
       "my-database": {
           "queries": {
               "add_twitter_handle": {
                   "sql": "insert into twitter_handles (username) values (:username)",
                   "write": true,
                   "allow": {
                       "id": ["simon"],
                       "role": ["staff"]
                   }
               }
           }
       }
    }
}

These get matched against the actor JSON. If any of the fields in any of the keys of "allow" match a key on the actor, the query is allowed.

"id": "*" matches any actor with an id key.

@simonw
Copy link
Owner Author

simonw commented Jun 5, 2020

I'd really like to support SQL query defined permissions too, mainly to set an example for how plugins could do something similar.

@simonw
Copy link
Owner Author

simonw commented Jun 5, 2020

Idea: an "allow_sql" key with a SQL query that gets passed the actor JSON as :actor and can extract the relevant keys from it and return 1 or 0.

@simonw
Copy link
Owner Author

simonw commented Jun 5, 2020

There can be a detailed section explaining these different mechanisms on the authentication documentation page.

I imagine they will end up applying to more than just canned queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant