Add parameter dialog doesn't work when query has selected text

[rauchy]

Issue Summary

Adding a parameter through the Parameter settings dialog won't work when you have a selected text in the query (parameter is added without any settings):

This is definitively not critical, but I thought it was worth to report in any case.

Steps to Reproduce

1. Open a query or create one
2. Select part of the text
3. Add a Parameter using Ctrl/Cmd+P
4. Notice that the parameter will be created without any settings

Technical details:

• Redash Version: Latest
• Browser/OS: --
• How did you install Redash:

[rauchy]

Dug around this a bit. The issue originates when a selection is made and a parameter is added, and in that case the Ace editor's replace function signals two changes - one when removing the text, and one when adding the new parameter fragment. (When there is no selection made - the range passed to replace is of 0 length, so no remove is triggered).

Each of these signals is caught and subsequently causes updateParameters to be called, in which a call to parseQuery() is made. The first call to parseQuery(), caused by the signal induced by removing the text, tries to parse the incomplete query, so it resets the parameters to default settings, according to the logic in updateParameters.

For example, for this query:

SELECT * FROM users WHERE id=5

If you select the "5" and create an "id" number parameter using the dialog, Ace editor will signal 2 changes:

SELECT * FROM users WHERE id=

and

SELECT * FROM users WHERE id={{id}}

The first signal will blank the query parameters (due to parseQuery), and the second will use those blanked settings.

A simple workaround for this would be to simply insert the parameter fragment (instead of replacing the selection) when a selection is made.

[ranbena]

Would debouncing parseQuery work?

[rauchy]

We'd have to debounce a higher function, because parseQuery has return values, which updateParameters relies on, and so forth.

I actually tried this with the source view's updateQuery function and it didn't work, but going back I see that I actually used throttle instead of debounce (I always confuse the two) and got pissed that it didn't work :)

I'm a little concerned of edge cases so I'm going to submit this as a PR and would appreciate any thoughts on where this might fudge things up.

[kravets-levko]

👍

[gabrieldutra]

👌

[gabrieldutra]

I can't approve the PR 😅 (I guess it's because I initially created the issue and now the issue is a PR).

In any case, LGTM! :)