-
Notifications
You must be signed in to change notification settings - Fork 237
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
feat: validation now works for templated query #1119
Conversation
czgu
commented
Jan 6, 2023
•
edited
Loading
edited
- fixed an issue where templated_variables kept getting regenerated
- make templated queries also get validated
- make templated query render modal also show validation errors with highlights
@@ -6,7 +6,6 @@ import { | |||
ILinterWarning, | |||
TableToken, | |||
} from 'lib/sql-helper/sql-lexer'; | |||
import { isQueryUsingTemplating } from 'lib/templated-query/validation'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this util function is only used in this file. As it's not needed here, it can be deleted.
@@ -41,6 +104,13 @@ export const TemplatedQueryView: React.FC<ITemplatedQueryViewProps> = ({ | |||
) | |||
); | |||
|
|||
const { validationErrorHighlights, queryValidationErrors, isValidating } = | |||
useValidateQuery( | |||
renderedQuery, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of 2 api calls here, I'm wondering if it'll be better to have the renderTemplatedQuery
api to accept another parameter to indicate if it should also do the validation on the rendered query and return the validation result along with the rendered query?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm I don't want to tie the two API calls together too much. One is for rendering the template and the other is to do validation check (only if the validator is configured). Also in future, I am thinking that we can make template rendering different if it is for validation so that we can map back the validation result from rendered query to unrendered query
* feat: validation now works for templated query * remove isQueryUsingTemplating
* feat: validation now works for templated query * remove isQueryUsingTemplating