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

Plugin hook: filters_from_request #473

Closed
simonw opened this issue May 19, 2019 · 13 comments
Closed

Plugin hook: filters_from_request #473

simonw opened this issue May 19, 2019 · 13 comments

Comments

@simonw
Copy link
Owner

simonw commented May 19, 2019

I meant to add this as part of the facets plugin mechanism but didn't quite get to it. Original idea was to allow plugins to register extra filters, as seen in datasette/filters.py:

class Filters:
_filters = (
[
# key, display, sql_template, human_template, format=, numeric=, no_argument=
TemplatedFilter(
"exact",
"=",
'"{c}" = :{p}',
lambda c, v: "{c} = {v}" if v.isdigit() else '{c} = "{v}"',
),
TemplatedFilter(
"not",
"!=",
'"{c}" != :{p}',
lambda c, v: "{c} != {v}" if v.isdigit() else '{c} != "{v}"',
),

@simonw
Copy link
Owner Author

simonw commented May 19, 2019

This expands on the refactoring work from 6da567d

@simonw
Copy link
Owner Author

simonw commented May 28, 2019

It would be nice if this plugin was passed the current database/table so it can decide to enable new filters only for specific tables. This will require a bit of refactoring because the filters list is static at the moment - it would instead have to be returned by a function that runs when the table view is rendered.

@simonw
Copy link
Owner Author

simonw commented May 28, 2019

I wonder if this is the right hook?

The more likely case is that we need a hook that registers a new type of lookup entirely - ?_spatial_within={geojson} for example. These aren't necessarily attached to a specific column and may have entirely custom syntax - but they still need to be able to influence the query (like the _where= clause does) and the human-readable description of the page.

Is there a strong case for supporting both custom filter plugins AND custom table where plugins, or could those where plugins cover both bases?

@simonw
Copy link
Owner Author

simonw commented May 28, 2019

I'm having trouble coming up with interesting column-based filters which don't make sense to ship as default behaviour.

@simonw
Copy link
Owner Author

simonw commented May 28, 2019

Assuming I do go ahead with this plugin hook, the existing InFilter makes for a nice simple example that illustrates the two key methods: .where_clause() and .human_clause():

class InFilter(Filter):
    key = "in"
    display = "in"

    def split_value(self, value):
        if value.startswith("["):
            return json.loads(value)
        else:
            return [v.strip() for v in value.split(",")]

    def where_clause(self, table, column, value, param_counter):
        values = self.split_value(value)
        params = [":p{}".format(param_counter + i) for i in range(len(values))]
        sql = "{} in ({})".format(escape_sqlite(column), ", ".join(params))
        return sql, values

    def human_clause(self, column, value):
        return "{} in {}".format(column, json.dumps(self.split_value(value)))

@simonw
Copy link
Owner Author

simonw commented May 28, 2019

Here's an older version of what that custom table filtering plugin might look like: 5116c4e

from datasette.utils import TableFilter

@hookimpl
def table_filter():
    async def inner(view, name, table, request):
        extra_human_descriptions = []
        where_clauses = []
        params = {}
        # ... build those things here
        return TableFilter(
            human_description_extras=extra_human_descriptions,
            where_clauses=where_clauses,
            params=params,
        )
    return inner

I built this for the https://github.com/simonw/russian-ira-facebook-ads-datasette project.

It's pretty neat. Maybe I should go with that?

@simonw
Copy link
Owner Author

simonw commented May 28, 2019

I'm leaning towards supporting both hooks.

@simonw
Copy link
Owner Author

simonw commented Dec 16, 2021

I revisited this idea in #1518 and came up with a slightly different name and design for the hook:

@hookspec
def filters_from_request(request, database, table, datasette):
    """
    Return FilterArguments(
        where_clauses=[str, str, str],
        params={},
        human_descriptions=[str, str, str],
        extra_context={}
    ) based on the request"""

@simonw simonw changed the title Plugin hook: register_filters Plugin hook: filters_from_request Dec 16, 2021
@simonw
Copy link
Owner Author

simonw commented Dec 16, 2021

This filter design can only influence the where component of the SQL clause - it's not able to modify the SELECT columns or adjust the ORDER BY or OFFSET LIMIT parts. I think that's OK.

simonw added a commit that referenced this issue Dec 16, 2021
New plugin hook, refs #473

Used it to extract the logic from TableView that handles _search and
_through and _where - refs #1518
@simonw
Copy link
Owner Author

simonw commented Dec 17, 2021

@simonw
Copy link
Owner Author

simonw commented Dec 17, 2021

I could use this hook to add table filtering on a map to the existing datasette-leaflet-freedraw plugin.

@simonw
Copy link
Owner Author

simonw commented Dec 17, 2021

The one slightly weird thing about this hook is how it adds extra_context without an obvious way for plugins to add extra HTML to the templates based on that context.

Maybe I need the proposed mechanism from

Which has an in-progress PR:

@simonw
Copy link
Owner Author

simonw commented Dec 17, 2021

I'm happy with how the prototype that used this plugin in datasette-leaflet-freedraw turned out: https://github.com/simonw/datasette-leaflet-freedraw/blob/e8a16a0fe90656b8d655c02881d23a2b9833281d/datasette_leaflet_freedraw/__init__.py

@simonw simonw closed this as completed in aa7f003 Dec 17, 2021
simonw added a commit that referenced this issue Dec 17, 2021
simonw added a commit to simonw/datasette-leaflet-freedraw that referenced this issue Dec 17, 2021
simonw added a commit that referenced this issue Jan 14, 2022
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