Edit query parameters on Ecto Stats page - #492
Conversation
The `_extras` modules used to power this form supply queries that sometimes offer parameters to customize the query. This adds a form to the Ecto Stats page that lets you change the values of parameters. For some databases like Cockroach DB, to get table statistics, you have to supply a table name, which made this sort of interaction necessary to make that query useful. This page is powered by a new "parameters" key in the info return value from the `c:info` callback on the `_extras` module. This is a new key to allow backwards compatibility with `_extras` modules that have not implemented this. I noticed that the exists modules all use string interpolation instead of bind parameters, so the Ecto Stats page uses the new "parameters" key as a sentinenl to know if the parameter form should be shown. I'll update the other modules to use bind parameters and add the "parameters" key to the `c:info` callback. Screenshots of the form are attached to the pull request. Please let me know what you think, thank you!
|
Shows an error mesage when the query fails instead of just crashing After filling in form Shows number input Shows boolean input |
|
Apologies, forgot I had some broken tests before I left for a long weekend last week. Converting to draft until I have those fixed up. |
|
I also removed OTP 24 from the test suite because that Elixir version was not compiling postgrex, and in #490 you mentioned not supporting OTP 24 any longer. |
| {:noreply, push_patch(socket, to: to)} | ||
| end | ||
|
|
||
| def handle_event("toggle_parameter_form", _params, socket) do |
There was a problem hiding this comment.
Can be done with JS events rather than server side?
There was a problem hiding this comment.
I wanted it to get persisted in the URL, which (to my knowledge) needs to go through the server. Please correct me if there is a better way to do this.
But if that is not desired, I can just remove the persistence.
There was a problem hiding this comment.
Also, I put it in a details tag just to make it less obtrusive, but I can just get rid of that entirely too if that's desired (basically always show it)
| |> Map.put_new(:row_attrs, []) | ||
| |> Map.put_new(:hint, nil) | ||
| |> Map.put_new(:dom_id, nil) | ||
| |> Map.put_new(:toolbar, []) |
There was a problem hiding this comment.
Does this really belong in the table component? Should we render it on the page instead of making it part of tables?
There was a problem hiding this comment.
I wanted the parameter form to go between the table title and the table itself, which required modifying the table component, or faking it by giving the table a nil title and then manually rendering a title on the page.
I'm open to an alternative if this new slot isn't desired.
2 participants
The
_extrasmodules used to power this form supply queries thatsometimes offer parameters to customize the query. This adds a form to
the Ecto Stats page that lets you change the values of parameters.
For some databases like Cockroach DB, to get table statistics, you have
to supply a table name, which made this sort of interaction necessary to
make that query useful.
This page is powered by a new "parameters" key in the info return value
from the
c:infocallback on the_extrasmodule. This is a new key toallow backwards compatibility with
_extrasmodules that have notimplemented this.
I noticed that the exists modules all use string interpolation instead
of bind parameters, so the Ecto Stats page uses the new "parameters" key
as a sentinenl to know if the parameter form should be shown. I'll
update the other modules to use bind parameters and add the "parameters"
key to the
c:infocallback.Screenshots of the form are attached to the pull request.
Please let me know what you think, thank you!