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

Alerting: Allow inserting before or after existing policy #83704

Merged
merged 6 commits into from
Mar 12, 2024

Conversation

gillesdemey
Copy link
Member

@gillesdemey gillesdemey commented Feb 29, 2024

What is this feature?

This PR introduces the ability to insert a new policy before or after an existing notification policy.

We currently do not have the ability to re-order notification policies and prior attempts have shown to have some rough edges; re-ordering nodes in large trees was slow, not accessible and hard to figure out now that pagination has been added to the policy tree.

Instead this PR adds the ability to choose where to insert new policies.

A move operation would be a delete + insert at position, with the caveat that label matchers would have to be copied / memorised by the user somewhere.

We feel like this is an acceptable compromise considering that operations on the tree wrt to moving the order of policies is quite rare and often required because the policy they had just inserted got appended to the bottom of the child policies.

image

Which issue(s) does this PR fix?:

Fixes #60101

Special notes for your reviewer:

Needs a few tests before merging to make sure the insertBefore / insertAfter functions work as intended.

Please check that:

@gillesdemey gillesdemey added this to the 11.0.x milestone Feb 29, 2024
@gillesdemey gillesdemey self-assigned this Feb 29, 2024
@gillesdemey gillesdemey changed the title Alerting: Allow inserting before after existing policy Alerting: Allow inserting before or after existing policy Feb 29, 2024
@gillesdemey gillesdemey marked this pull request as ready for review February 29, 2024 17:10
@gillesdemey gillesdemey requested a review from a team as a code owner February 29, 2024 17:10
@gillesdemey gillesdemey requested review from konrad147 and soniaAguilarPeiron and removed request for a team February 29, 2024 17:10
@gillesdemey gillesdemey requested review from a team as code owners March 7, 2024 15:19
@gillesdemey gillesdemey requested review from tskarhed, JoaoSilvaGrafana and mckn and removed request for a team March 7, 2024 15:19
Copy link
Member

@soniaAguilarPeiron soniaAguilarPeiron left a comment

Choose a reason for hiding this comment

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

LGTM!🚀
No nits! tested locally and it looks great!
Cool feature!

@gillesdemey gillesdemey merged commit 388e0c2 into main Mar 12, 2024
18 checks passed
@gillesdemey gillesdemey deleted the alerting/insert-policy-before-after branch March 12, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Alerting: Allow re-ordering of Notification Policies
3 participants