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

datasette.urls.table() / .instance() / .database() methods for constructing URLs, also exposed to templates #904

Closed
simonw opened this issue Jul 21, 2020 · 11 comments

Comments

@simonw
Copy link
Owner

simonw commented Jul 21, 2020

I tried using this block of template in a plugin and got an error:

{% block nav %}
    <p class="crumbs">
        <a href="{{ base_url }}">home</a> /
        <a href="{{ database_url(database) }}">{{ database }}</a> /
        <a href="{{ database_url(database) }}/{{ table|quote_plus }}">{{ table }}</a>
    </p>
    {{ super() }}
{% endblock %}

Error: 'database_url' is undefined

That's because database_url is only made available by the BaseView template here:

async def render(self, templates, request, context=None):
context = context or {}
template = self.ds.jinja_env.select_template(templates)
template_context = {
**context,
**{
"database_url": self.database_url,
"database_color": self.database_color,
"select_templates": [
"{}{}".format(
"*" if template_name == template.name else "", template_name
)
for template_name in templates
],
},
}

@simonw
Copy link
Owner Author

simonw commented Oct 15, 2020

Tracking ticket: #1023

@simonw
Copy link
Owner Author

simonw commented Oct 15, 2020

Here's the current implementation of database_url - on the BaseView class, but only because it needs access to a datasette instance (to read base_url):

def database_url(self, database):
db = self.ds.databases[database]
base_url = self.ds.config("base_url")
if self.ds.config("hash_urls") and db.hash:
return "{}{}-{}".format(base_url, database, db.hash[:HASH_LENGTH])
else:
return "{}{}".format(base_url, database)

@simonw
Copy link
Owner Author

simonw commented Oct 15, 2020

I think this should be a family of functions:

  • instance_url() - the root URL of the instance (usually / unless base_url is set)
  • database_url(database_name) - already got this
  • table_url(database_name, table_name)
  • row_url(database_name, table_name, row) - not sure about this one. The idea would be for row to be correctly turned into a URL by introspecting the primary keys for that table, then pulling those values out of the SQLite row object. Might not be necessary though.

I also need a way for plugins to link to e.g. /-/configure-fts - or even /-/configure-fts/database-name/table-name. What should that look like?

@simonw
Copy link
Owner Author

simonw commented Oct 15, 2020

Could have instance_url() take an optional path argument which is then turned into the correct path.

@simonw
Copy link
Owner Author

simonw commented Oct 16, 2020

Alternatively, I could expose a single object that knows how to construct all kinds of URLs. Something like this:

{{ urls.instance() }}
{{ urls.database(database_name) }}
{{ urls.table(database_name, table_name) }}
etc

@simonw
Copy link
Owner Author

simonw commented Oct 19, 2020

If I do these methods I think this should be available on the datasette object too, as an internal API for plugins to use to construct redirects and suchlike.

@simonw simonw pinned this issue Oct 19, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 19, 2020

Related: #1026 (How should datasette.client interact with base_url)

Also this comment from #943 (comment)

One thing to consider here: Datasette's table and database name escaping rules can be a little bit convoluted.

If a plugin wants to get back the first five rows of a table, it will need to construct a URL /dbname/tablename?_size=5 - but it will need to know how to turn the database and table names into the correctly escaped dbname and tablename values.

@simonw
Copy link
Owner Author

simonw commented Oct 19, 2020

Options:

  • datasette.urls.instance() (or datasette.urls.root()), datasette.urls.table() etc
  • datasette.url_for.instance()...
  • datasette.url.instance()...
  • datasette.root_url(), datasette.table_url()...

@simonw
Copy link
Owner Author

simonw commented Oct 19, 2020

I quite like datasette.urls.table() but I'm not sure why.

@simonw simonw changed the title Make database_url and other helpers available within .render_template() for plugins datasette.urls.table() / .instance() / .database() methods for constructing URLs, also exposed to templates Oct 19, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 19, 2020

OK, I'm committing to datasette.urls.table() and friends, which will be available to (and documented for use in) plugins and will be exposed to templates as {{ urls.table(...) }}.

simonw added a commit that referenced this issue Oct 20, 2020
simonw added a commit that referenced this issue Oct 20, 2020
@simonw simonw unpinned this issue Oct 20, 2020
@simonw simonw added this to the 0.51 milestone Oct 23, 2020
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