Skip to content
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

InfluxDB: Fix for wrong query generated with template variable and non regex operator on frontend mode #84175

Merged
merged 6 commits into from
Mar 21, 2024

Conversation

wasim-nihal
Copy link
Contributor

@wasim-nihal wasim-nihal commented Mar 11, 2024

What is this feature?

When using a single value regex variable with a non-regex operator (e.g. =), and you convert the query from builder to code editor, it leaves the regex wrapper around the variable in the expression, and wraps that in a single quote, which leads to a broken query expression. This PR is fixing it.

Why do we need this feature?

Better query generation while switching between editors

Who is this feature for?

InfluxDB users building InfluxQL queries on grafana

Which issue(s) does this PR fix?:

Fixes #81517

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
@wasim-nihal wasim-nihal requested a review from a team as a code owner March 11, 2024 11:52
@CLAassistant
Copy link

CLAassistant commented Mar 11, 2024

CLA assistant check
All committers have signed the CLA.

@itsmylife itsmylife changed the title fix for wrong query generated for influxdb with template variable and non regex operator. InfluxDB: Fix for wrong query generated with template variable and non regex operator on frontend mode Mar 21, 2024
@itsmylife itsmylife added this to the 11.0.x milestone Mar 21, 2024
@itsmylife
Copy link
Contributor

@wasim-nihal The PR looks good. But in the CI there are some errors. Could you please run yarn prettier:write and commit the changes?

@wasim-nihal
Copy link
Contributor Author

@wasim-nihal The PR looks good. But in the CI there are some errors. Could you please run yarn prettier:write and commit the changes?

sure.

Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
@wasim-nihal
Copy link
Contributor Author

Done. Could you please re-trigger?

Copy link
Contributor

@NWRichmond NWRichmond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! It appears to work as expected:

demo.fix.81517.mov

I made a few suggestions. Probably the most pressing one is fixing the infuxdb > InfluxDB spelling issues.

Copy link
Contributor

@itsmylife itsmylife left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Could you please check @NWRichmond 's comments too? Then I think we are good to go. Thanks for this contribution!

Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
@wasim-nihal
Copy link
Contributor Author

wasim-nihal commented Mar 21, 2024

@itsmylife , @NWRichmond , thanks for reviewing this change. I have addressed the comments in the latest commit.

@wasim-nihal
Copy link
Contributor Author

Seeing some linting issues again in CI. Will push the changes.

Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
@wasim-nihal
Copy link
Contributor Author

Seeing some linting issues again in CI. Will push the changes.

Done

@itsmylife
Copy link
Contributor

I triggered the build and waiting for it to start. Thanks for the fixes.

@itsmylife itsmylife merged commit 15122bc into grafana:main Mar 21, 2024
12 checks passed
@itsmylife
Copy link
Contributor

@wasim-nihal Thank you for the contribution once more! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants