search

simonw / django-sql-dashboard

Django app for building dashboards using raw SQL queries
https://django-sql-dashboard.datasette.io/
Apache License 2.0
437 stars 37 forks source link

Refactor named parameters feature to allow extensibility #148

Closed ipamo closed 2 years ago

ipamo commented 2 years ago

Refactor parsing of (and access to) named parameters into a single extensible Parameter class. The feature remains the same but the implementation is cleaner.

In addition, the actual Parameter class used by the library may be changed using a setting named DASHBOARD_PARAMETER_CLASS. This allows users of the library to implement custom parameter parsings without changing the default behaviour of the libary.

See issue #149 for extension ideas (%(value)d, %(flag:true)b, etc).

Note: I changed the id of the input HTML fields (from numerical qpX to qp_{parameterName}) so that these fields may be generated directly from instances of the Parameter class.

All tests pass.

simonw commented 2 years ago

This is really neat, thank you.

simonw commented 2 years ago

I'm afraid I'm going to have to revert this change back out again - I couldn't get the test suite to pass with it, and when I manually tested I got errors like this one:

CleanShot 2022-04-19 at 17 16 00@2x

I tracked that down to a KeyError being thrown in this block of code: https://github.com/simonw/django-sql-dashboard/blob/486736fbeaae08a6cc91fafa9ffefaeeef3e05e2/django_sql_dashboard/views.py#L244-L257