-
Notifications
You must be signed in to change notification settings - Fork 13.1k
DashboardList: Enable templating on search tag input #31460
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
Conversation
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.
@delta50 thanks for the contribution. I've tested it and it seems like it works as expected.
Few things before we can merge this.
- Docs needs to be updated (
### Options - Would be awesome if you could implement tests for this newly added feature
- Your co-committer needs to sign CLA agreement (andrew.riedel@deepintent.com - I assume it's you) :)
Additionally- since the change does not seem to be a big one - could you also add the variables interpolation to the search query input?
959756d
to
4cf782a
Compare
@dprokop I have added the variable interpolation to the query field, added documentation, and fixed the commit author. I'm not sure where the tests for this would live, or what kind of tests you had in mind. Could you provide more info on what this would look like? |
4cf782a
to
b4be2ce
Compare
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.
Added some copy edit suggestions, code changes will need a review from engineers.
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
const params = { | ||
limit: options.maxItems, | ||
query: options.query, | ||
query: /^\$.+/.test(options.query) ? replaceVars(options.query, {}, 'text') : options.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.
always pass it through replaceVars, there are multiple syntaxes so testing if the string contains variables is not needed
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.
This is done and the documentation has been updated.
This needs a milestone. Is it slated for 7.5? |
I would very much appreciate if this was able to go out in 7.5 as it is will help simplify our grafana setup significantly. If there is anything left to do let me know and I can knock it out ASAP. |
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.
One minor copy-edit suggestion. Otherwise doc content looks Ok.
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
Hey @torkelo, is this good to go or are there more changes required? |
Co-authored-by: Torkel Ödegaard <torkel@grafana.com>
* master: (168 commits) Fix loading timezone info on windows (grafana#32029) Remove datalink template suggestions for accessing specific fields when there are multiple dataframes. (grafana#32057) AlertingNG: Refactor notifier to support config reloads (grafana#32099) Chore: Replace Command dispatches by explicit calls (grafana#32131) MixedDataSource: Name is updated when data source variable changes (grafana#32090) Plugin: PanelRenderer and simplified QueryRunner to be used from plugins. (grafana#31901) DashboardList: Enable templating on search tag input (grafana#31460) release notes v7.3.10 and v7.4.5 (grafana#32135) Alerting: Add database table for persisting alerting configuration (grafana#32042) latest updated to 7.4.5 (grafana#32132) changelog v7.4.5 (grafana#32110) Chore: Tidy up Go deps (grafana#32127) Explore: Support full inspect drawer (grafana#32005) GraphNG: make sure dataset and config are in sync when initializing and re-initializing uPlot (grafana#32106) Login: Replace command dispatch by explicit call (grafana#32088) Loki: Label browser UI updates (grafana#31737) AlertingNG: Notification channel for emails (grafana#31768) Alerting: Enable Alert rule `severity` tag to override VictorOps Severity setting (grafana#29392) Docs: updates developer guide with useful resources for windows setup (grafana#32075) SQLStore: Close session in withDbSession (grafana#31775) ...
* Add variable interpolation for dashlist panel. * Update docs/sources/panels/visualizations/dashboard-list-panel.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Fix variable interpolation for dashlist panel to work with all syntax. * Update docs/sources/panels/visualizations/dashboard-list-panel.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Fix prettier issues. * Fix prettier issues. * Update docs/sources/panels/visualizations/dashboard-list-panel.md Co-authored-by: Torkel Ödegaard <torkel@grafana.com> Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> Co-authored-by: Torkel Ödegaard <torkel@grafana.com>
What this PR does / why we need it:
Now it is possible to pass variables to the search tag input field of the dashlist config. This is useful when you have a generic dashboard and would like to use the templating feature for showing only the relevant dashboards in the list depending on a variable value.
Which issue(s) this PR fixes:
Fixes: #29028
Relates #18113 #18167
Special notes for your reviewer:
This is an updated version of this outdated PR #18167