From 4fa7cf68536628344356d3ef8c92c25c249067a0 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 2 Jun 2020 14:08:12 -0700 Subject: [PATCH] Flash messages mechanism, closes #790 --- datasette/app.py | 42 +++++++++++++++++++++++ datasette/static/app.css | 16 +++++++++ datasette/templates/base.html | 8 +++++ datasette/templates/messages_debug.html | 26 ++++++++++++++ datasette/utils/asgi.py | 4 +-- datasette/views/base.py | 16 +++++++++ datasette/views/special.py | 24 +++++++++++++ docs/internals.rst | 18 ++++++++++ docs/introspection.rst | 8 +++++ tests/fixtures.py | 6 ++++ tests/plugins/messages_output_renderer.py | 21 ++++++++++++ tests/test_api.py | 1 + tests/test_auth.py | 6 +--- tests/test_messages.py | 28 +++++++++++++++ 14 files changed, 217 insertions(+), 7 deletions(-) create mode 100644 datasette/templates/messages_debug.html create mode 100644 tests/plugins/messages_output_renderer.py create mode 100644 tests/test_messages.py diff --git a/datasette/app.py b/datasette/app.py index e3ad5fc776..41c73900ea 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -2,6 +2,7 @@ import collections import datetime import hashlib +from http.cookies import SimpleCookie import itertools import json import os @@ -30,6 +31,7 @@ PatternPortfolioView, AuthTokenView, PermissionsDebugView, + MessagesDebugView, ) from .views.table import RowView, TableView from .renderer import json_renderer @@ -156,6 +158,11 @@ async def favicon(scope, receive, send): class Datasette: + # Message constants: + INFO = 1 + WARNING = 2 + ERROR = 3 + def __init__( self, files, @@ -423,6 +430,38 @@ def _prepare_connection(self, conn, database): # pylint: disable=no-member pm.hook.prepare_connection(conn=conn, database=database, datasette=self) + def add_message(self, request, message, type=INFO): + if not hasattr(request, "_messages"): + request._messages = [] + request._messages_should_clear = False + request._messages.append((message, type)) + + def _write_messages_to_response(self, request, response): + if getattr(request, "_messages", None): + # Set those messages + cookie = SimpleCookie() + cookie["ds_messages"] = self.sign(request._messages, "messages") + cookie["ds_messages"]["path"] = "/" + # TODO: Co-exist with existing set-cookie headers + assert "set-cookie" not in response.headers + response.headers["set-cookie"] = cookie.output(header="").lstrip() + elif getattr(request, "_messages_should_clear", False): + cookie = SimpleCookie() + cookie["ds_messages"] = "" + cookie["ds_messages"]["path"] = "/" + # TODO: Co-exist with existing set-cookie headers + assert "set-cookie" not in response.headers + response.headers["set-cookie"] = cookie.output(header="").lstrip() + + def _show_messages(self, request): + if getattr(request, "_messages", None): + request._messages_should_clear = True + messages = request._messages + request._messages = [] + return messages + else: + return [] + async def permission_allowed( self, actor, action, resource_type=None, resource_identifier=None, default=False ): @@ -808,6 +847,9 @@ def add_route(view, regex): add_route( PermissionsDebugView.as_asgi(self), r"/-/permissions$", ) + add_route( + MessagesDebugView.as_asgi(self), r"/-/messages$", + ) add_route( PatternPortfolioView.as_asgi(self), r"/-/patterns$", ) diff --git a/datasette/static/app.css b/datasette/static/app.css index 92f268ae6f..774a223513 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -351,3 +351,19 @@ p.zero-results { .type-float, .type-int { color: #666; } + +.message-info { + padding: 1em; + border: 1px solid green; + background-color: #c7fbc7; +} +.message-warning { + padding: 1em; + border: 1px solid #ae7100; + background-color: #fbdda5; +} +.message-error { + padding: 1em; + border: 1px solid red; + background-color: pink; +} diff --git a/datasette/templates/base.html b/datasette/templates/base.html index d9fd945b9a..9b871d03a3 100644 --- a/datasette/templates/base.html +++ b/datasette/templates/base.html @@ -17,6 +17,14 @@
+{% block messages %} +{% if show_messages %} + {% for message, message_type in show_messages() %} +

{{ message }}

+ {% endfor %} +{% endif %} +{% endblock %} + {% block content %} {% endblock %}
diff --git a/datasette/templates/messages_debug.html b/datasette/templates/messages_debug.html new file mode 100644 index 0000000000..b2e1bc7c85 --- /dev/null +++ b/datasette/templates/messages_debug.html @@ -0,0 +1,26 @@ +{% extends "base.html" %} + +{% block title %}Debug messages{% endblock %} + +{% block content %} + +

Debug messages

+ +

Set a message:

+ +
+
+ +
+ +
+ +
+
+ +{% endblock %} diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index 960532ca5a..5682da4802 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -180,9 +180,9 @@ async def __call__(self, scope, receive, send): class AsgiView: - def dispatch_request(self, request, *args, **kwargs): + async def dispatch_request(self, request, *args, **kwargs): handler = getattr(self, request.method.lower(), None) - return handler(request, *args, **kwargs) + return await handler(request, *args, **kwargs) @classmethod def as_asgi(cls, *class_args, **class_kwargs): diff --git a/datasette/views/base.py b/datasette/views/base.py index 06b78d5f43..2402406a4a 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -1,6 +1,7 @@ import asyncio import csv import itertools +from itsdangerous import BadSignature import json import re import time @@ -73,6 +74,20 @@ def database_url(self, database): def database_color(self, database): return "ff0000" + async def dispatch_request(self, request, *args, **kwargs): + # Populate request_messages if ds_messages cookie is present + if self.ds: + try: + request._messages = self.ds.unsign( + request.cookies.get("ds_messages", ""), "messages" + ) + except BadSignature: + pass + response = await super().dispatch_request(request, *args, **kwargs) + if self.ds: + self.ds._write_messages_to_response(request, response) + return response + async def render(self, templates, request, context=None): context = context or {} template = self.ds.jinja_env.select_template(templates) @@ -81,6 +96,7 @@ async def render(self, templates, request, context=None): **{ "database_url": self.database_url, "database_color": self.database_color, + "show_messages": lambda: self.ds._show_messages(request), "select_templates": [ "{}{}".format( "*" if template_name == template.name else "", template_name diff --git a/datasette/views/special.py b/datasette/views/special.py index 811ed4cb2e..37c0469724 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -94,3 +94,27 @@ async def get(self, request): request, {"permission_checks": reversed(self.ds._permission_checks)}, ) + + +class MessagesDebugView(BaseView): + name = "messages_debug" + + def __init__(self, datasette): + self.ds = datasette + + async def get(self, request): + return await self.render(["messages_debug.html"], request) + + async def post(self, request): + post = await request.post_vars() + message = post.get("message", "") + message_type = post.get("message_type") or "INFO" + assert message_type in ("INFO", "WARNING", "ERROR", "all") + datasette = self.ds + if message_type == "all": + datasette.add_message(request, message, datasette.INFO) + datasette.add_message(request, message, datasette.WARNING) + datasette.add_message(request, message, datasette.ERROR) + else: + datasette.add_message(request, message, getattr(datasette, message_type)) + return Response.redirect("/") diff --git a/docs/internals.rst b/docs/internals.rst index b3ad623f53..4d51d6148f 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -214,6 +214,24 @@ This method returns a signed string, which can be decoded and verified using :re Returns the original, decoded object that was passed to :ref:`datasette_sign`. If the signature is not valid this raises a ``itsdangerous.BadSignature`` exception. +.. _datasette_add_message: + +.add_message(request, message, message_type=datasette.INFO) +----------------------------------------------------------- + +``request`` - Request + The current Request object + +``message`` - string + The message string + +``message_type`` - constant, optional + The message type - ``datasette.INFO``, ``datasette.WARNING`` or ``datasette.ERROR`` + +Datasette's flash messaging mechanism allows you to add a message that will be displayed to the user on the next page that they visit. Messages are persisted in a ``ds_messages`` cookie. This method adds a message to that cookie. + +You can try out these messages (including the different visual styling of the three message types) using the ``/-/messages`` debugging tool. + .. _internals_database: Database class diff --git a/docs/introspection.rst b/docs/introspection.rst index e5d08dbc05..084ee144a4 100644 --- a/docs/introspection.rst +++ b/docs/introspection.rst @@ -166,3 +166,11 @@ Shows the currently authenticated actor. Useful for debugging Datasette authenti "username": "some-user" } } + + +.. _MessagesDebugView: + +/-/messages +----------- + +The debug tool at ``/-/messages`` can be used to set flash messages to try out that feature. See :ref:`datasette_add_message` for details of this feature. diff --git a/tests/fixtures.py b/tests/fixtures.py index b2cfd3d659..daff016880 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -29,6 +29,12 @@ def __init__(self, status, headers, body): self.headers = headers self.body = body + @property + def cookies(self): + cookie = SimpleCookie() + cookie.load(self.headers.get("set-cookie") or "") + return {key: value.value for key, value in cookie.items()} + @property def json(self): return json.loads(self.text) diff --git a/tests/plugins/messages_output_renderer.py b/tests/plugins/messages_output_renderer.py new file mode 100644 index 0000000000..6b52f8011e --- /dev/null +++ b/tests/plugins/messages_output_renderer.py @@ -0,0 +1,21 @@ +from datasette import hookimpl + + +def render_message_debug(datasette, request): + if request.args.get("add_msg"): + msg_type = request.args.get("type", "INFO") + datasette.add_message( + request, request.args["add_msg"], getattr(datasette, msg_type) + ) + return {"body": "Hello from message debug"} + + +@hookimpl +def register_output_renderer(datasette): + return [ + { + "extension": "message", + "render": render_message_debug, + "can_render": lambda: False, + } + ] diff --git a/tests/test_api.py b/tests/test_api.py index d7e7c03f5e..a5c6f6a2c2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1262,6 +1262,7 @@ def test_plugins_json(app_client): expected = [ {"name": name, "static": False, "templates": False, "version": None} for name in ( + "messages_output_renderer.py", "my_plugin.py", "my_plugin_2.py", "register_output_renderer.py", diff --git a/tests/test_auth.py b/tests/test_auth.py index ddf328af2d..ac8d7abec2 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -9,11 +9,7 @@ def test_auth_token(app_client): response = app_client.get(path, allow_redirects=False,) assert 302 == response.status assert "/" == response.headers["Location"] - set_cookie = response.headers["set-cookie"] - assert set_cookie.endswith("; Path=/") - assert set_cookie.startswith("ds_actor=") - cookie_value = set_cookie.split("ds_actor=")[1].split("; Path=/")[0] - assert {"id": "root"} == app_client.ds.unsign(cookie_value, "actor") + assert {"id": "root"} == app_client.ds.unsign(response.cookies["ds_actor"], "actor") # Check that a second with same token fails assert app_client.ds._root_token is None assert 403 == app_client.get(path, allow_redirects=False,).status diff --git a/tests/test_messages.py b/tests/test_messages.py new file mode 100644 index 0000000000..d17e015cb1 --- /dev/null +++ b/tests/test_messages.py @@ -0,0 +1,28 @@ +from .fixtures import app_client +import pytest + + +@pytest.mark.parametrize( + "qs,expected", + [ + ("add_msg=added-message", [["added-message", 1]]), + ("add_msg=added-warning&type=WARNING", [["added-warning", 2]]), + ("add_msg=added-error&type=ERROR", [["added-error", 3]]), + ], +) +def test_add_message_sets_cookie(app_client, qs, expected): + response = app_client.get("/fixtures.message?{}".format(qs)) + signed = response.cookies["ds_messages"] + decoded = app_client.ds.unsign(signed, "messages") + assert expected == decoded + + +def test_messages_are_displayed_and_cleared(app_client): + # First set the message cookie + set_msg_response = app_client.get("/fixtures.message?add_msg=xmessagex") + # Now access a page that displays messages + response = app_client.get("/", cookies=set_msg_response.cookies) + # Messages should be in that HTML + assert "xmessagex" in response.text + # Cookie should have been set that clears messages + assert "" == response.cookies["ds_messages"]