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.

Sign up for GitHub

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

Jump to bottom

Alerting: Filter rules list #32818

Merged
merged 16 commits into from Apr 15, 2021
Merged

Alerting: Filter rules list #32818

merged 16 commits into from Apr 15, 2021

Conversation

nathanrodman
Copy link
Contributor

Screen.Recording.2021-04-08.at.4.11.09.PM.mov

What this PR does / why we need it:

Filters the rules for System/Application rules. Adds the filters to query params so that links are shareable.

Which issue(s) this PR fixes:

https://github.com/grafana/alerting-squad/issues/24

Fixes #

Special notes for your reviewer:

domasx2
domasx2 reviewed Apr 9, 2021
View reviewed changes
Copy link
Contributor

@domasx2 domasx2 left a comment

Choose a reason for hiding this comment

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

great progress, left some comments 👍

public/app/features/alerting/unified/components/rule-editor/AlertTypeSection.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/components/DataSourceSelect.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/components/DataSourceSelect.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/components/DataSourceSelect.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/components/rules/RulesFilter.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/components/rules/RulesFilter.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/components/rules/RulesFilter.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/components/rules/RulesFilter.tsx Outdated Show resolved Hide resolved
@nathanrodman nathanrodman force-pushed the filter-rules-list branch 2 times, most recently from dd03181 to 7e94612 Compare April 10, 2021 01:20
@nathanrodman nathanrodman marked this pull request as ready for review April 12, 2021 15:12
@nathanrodman nathanrodman requested review from a team and torkelo and removed request for a team April 12, 2021 15:12
domasx2
domasx2 requested changes Apr 12, 2021
View reviewed changes
Copy link
Contributor

@domasx2 domasx2 left a comment

Choose a reason for hiding this comment

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

see comments :)

public/app/features/alerting/unified/components/rules/RulesFilter.tsx Show resolved Hide resolved
public/app/features/alerting/unified/components/rules/RulesFilter.tsx Show resolved Hide resolved
public/app/features/alerting/unified/components/rules/SystemOrApplicationRules.tsx Outdated Show resolved Hide resolved
public/app/features/alerting/unified/hooks/useFilteredRules.ts Show resolved Hide resolved
domasx2
domasx2 approved these changes Apr 13, 2021
View reviewed changes
Copy link
Contributor

@domasx2 domasx2 left a comment

Choose a reason for hiding this comment

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

I pushed a few very minor fixes, good to merge imho!

@peterholmberg peterholmberg requested a review from mckn April 13, 2021 07:51
peterholmberg
peterholmberg requested changes Apr 13, 2021
View reviewed changes
Copy link
Contributor

@peterholmberg peterholmberg left a comment

Choose a reason for hiding this comment

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

I added a comment about a function that is hard to reason about.

Also noticed that the list doesn't update until the polling when clearing the search box.

Alerting_.Alert.rules.-.Grafana.-.Google.Chrome.2021-04-13.11-17-20.mp4

@nathanrodman
Copy link
Contributor Author

Thanks for catching that @peterholmberg I'll take a look as to why it's not updating with the filter change, it's set as a dependency in the hook but the debouncing may be affecting it

@domasx2
Copy link
Contributor

domasx2 commented Apr 14, 2021

I added a comment about a function that is hard to reason about.

Also noticed that the list doesn't update until the polling when clearing the search box.

Alerting_.Alert.rules.-.Grafana.-.Google.Chrome.2021-04-13.11-17-20.mp4

fixed!

peterholmberg
peterholmberg approved these changes Apr 15, 2021
View reviewed changes
Copy link
Contributor

@peterholmberg peterholmberg left a comment

Choose a reason for hiding this comment

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

Looks good now

@domasx2 domasx2 merged commit 345d9f9 into master Apr 15, 2021
@domasx2 domasx2 deleted the filter-rules-list branch April 15, 2021 11:53
davidmparrott added a commit that referenced this pull request Apr 15, 2021
@davidmparrott @torkelo
@kaydelaney @ivanahuckova @dafydd-t @n-wbrown @thisisobate @leeoniya @codesome @dprokop @nathanrodman @hugohaggmark @papagian @gotjosh @marefr
Alerting: set query in rules response (#33010)
2276e95 
* set query in rules response

* Theme: tweaking dark theme colors (#33007)

* Library Panels: Add library panel tab to share modal (#32953)

* Explore: Scroll split panes in Explore independently (#32978)

* Change default prometheus to latest and prometheus v1 to prometheus1

* Update README

* Remove prometheus1 block as not used

* Explore: Separatae scrolling in split view

* Update snapshot

* Allow skip migrations in tests via environment variable (#32958)

* Dashboard: Fix issue where Slack notifications won't link to users (#32861)

* DashboardPage: refactored styles from sass to emotion (#32955)

* DashboardPage: refactored styles from sass to emotion

* refactored dashboardPage component to be alot easier to read and understand

* more refactoring...

* more cleaning...

* fixes frontend test

* fixes frontend test- I hope

* fixes frontend test- I hope

* moves dashboard scss styles back to it's standalone file

* GraphNG: use theme font family and size for axis labels (#33009)

* GraphNG: use theme font family and size for axis labels

* fix test

* AlertingNG: Slack notification channel (#32675)

* AlertingNG: Slack notification channel

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Add tests

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments and small refactoring

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* GraphNG: stacking (#30749)

* First iteration

* Dev dash

* Re-use StackingMode type

* Fix ts and api issues

* Stacking work resurected

* Fix overrides

* Correct values in tooltip and updated test dashboard

* Update dev dashboard

* Apply correct bands for stacking

* Merge fix

* Update snapshot

* Revert go.sum

* Handle null values correctyl and make filleBelowTo and stacking mutual exclusive

* Snapshots update

* Graph->Time series stacking migration

* Review comments

* Indicate overrides in StandardEditorContext

* Change stacking UI editor, migrate stacking to object option

* Small refactor, fix for hiding series and dev dashboard

* VizLegend: sets a min and max value of the seriesCount control in Storybook (#33022)

* Alerting: Filter rules list (#32818)

* Chore: Reduces strict errors (#33012)

* Chore: reduces strict error in OptionPicker tests

* Chore: reduces strict errors in FormDropdownCtrl

* Chore: reduces has no initializer and is not definitely assigned in the constructor errors

* Chore: reduces has no initializer and is not definitely assigned in the constructor errors

* Chore: lowers strict count limit

* Tests: updates snapshots

* Tests: updates snapshots

* Chore: updates after PR comments

* Refactor: removes throw and changes signature for DashboardSrv.getCurrent

* [Alerting]: Several modifications in alert rules (#32983)

* [Alerting]: Use common properties for all rules

* Add Labels in rules

* Fix update ruleGroup API

Return 400 Bad Request response
when the request contains a UID that does not exist

* Check permissions and return namespace id

* Apply suggestions from code review

Co-authored-by: gotjosh <josue@grafana.com>

* WIP (#33025)

* Chore: Bump strict error count limit (#33035)

* set query in rules response

Co-authored-by: Torkel Ödegaard <torkel@grafana.org>
Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>
Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com>
Co-authored-by: Dafydd <72009875+dafydd-t@users.noreply.github.com>
Co-authored-by: n-wbrown <n-wbrown@users.noreply.github.com>
Co-authored-by: Uchechukwu Obasi <obasiuche62@gmail.com>
Co-authored-by: Leon Sorokin <leeoniya@gmail.com>
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
Co-authored-by: Nathan Rodman <nathanrodman@gmail.com>
Co-authored-by: Hugo Häggmark <hugo.haggmark@grafana.com>
Co-authored-by: Sofia Papagiannaki <papagian@users.noreply.github.com>
Co-authored-by: gotjosh <josue@grafana.com>
Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domasx2 domasx2 domasx2 approved these changes

@peterholmberg peterholmberg peterholmberg approved these changes

@torkelo torkelo Awaiting requested review from torkelo

@mckn mckn Awaiting requested review from mckn

Assignees
No one assigned
Labels
area/backend area/frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nathanrodman @domasx2 @torkelo @peterholmberg @grafanabot