Query Parameter validation and feedback

In #2721 it was suggested to display more informative and user friendly feedback when query results fail due to missing parameter value.

But parameter validation can be made even broader as there are other validations being done in the backend which can emit dedicated significant feedback.

In #4264 I attempt to adhere to all validation cases and here they are up for discussion.

1. Missing value

When a parameter has no value.


2. Invalid value

Numbers for Number params, string for Text params, object for DateRange params etc.

In the following screenshot, a valid DateRange param was changed to a Text param which doesn’t accept an object as a value.


3. Missing parameter

When a widget executes a query which has since been added a new parameter.


4. Redundant parameter

When a widget executes a query with a parameter that has already been removed.


5. Mismatched parameter

When there’s a mismatch between the parameter type and the expected template tag in the query text.

When a DateRange param is missing {{ param.start }} or {{ param.end }}.

Same for the opposite case, when a non-DateRange param is missing the normal {{ param }}.

But if the same mismatch happens when the query is not saved - meaning, no schema to validate by parameter type - a more generic error is shown, suggesting to save the query and rerun.


6. Unsafe parameter

When there’s an unsafe parameter in a widget of a limited access public dashboard.


7. Unsaved parameter

This one is not really related but I felt the parameter feedback mechanism could be used to show this as well.

When a new parameter has been added but query not yet saved.

Lmk what you think.

3 Likes

Excellent usability improvement. I was just lamenting the lack of this sort of contextual errors on parameters last night. I feel like building Redash into our project is strapping us to a rocket engine. Keep it up!

5 Likes

I’m not sure we need to change the current behavior. Because the user can’t do anything about it, it’s enough just to show the message.

When showing this it’s unclear for the user what to do. Maybe show a hover message saying “Click on Save”?

Are you sure this generates an error?

  • Should be Required Parameter.
  • I wonder if just a simple asterisk is enough and show the full message on hover? It just that some of those messages are long.

Thanks, @chrismerck :pray: Appreciate the feedback.

I’m not sure we need to change the current behavior. Because the user can’t do anything about it, it’s enough just to show the message.

It’s mostly valuable for unsafe dashboard parameters, in which it is harder to correlate between a failing widget and its actual unsafe parameter.
That said, it’s not a must so I’ll remove the implementation.

Maybe show a hover message saying “Click on Save”?

:+1:

Are you sure this generates an error?

Yes. Due to this param check:

But since having a redundant parameter won’t fail the query, it can be ignored - how about removing this check altogether?

Should be Required Parameter

:+1:

I wonder if just a simple asterisk is enough and show the full message on hover? It just that some of those messages are long.

I’ll give it a try.

This check is needed, but what we can do is remove any parameter value that doesn’t have a matching parameter. There is no harm in ignoring them this way. @rauchy what do you think?