-
Notifications
You must be signed in to change notification settings - Fork 0
Notification Rule Processing Engine #6
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
base: notification-rule-baseline
Are you sure you want to change the base?
Notification Rule Processing Engine #6
Conversation
* wip * Add working actions for GMA rules based on Prom-only API * Remove Ruler-loader related code for Grafana rules Co-authored-by: Sonia Augilar <[email protected]> * Remove outdated tests * add some comments * remove commented code * remove showLocation property * Add missing mocks in tests * Add showLocation to GrafanaRuleListItem, improve useAbilities, address PR feedback * Enhance GrafanaGroupLoader tests: Add permission checks and More button functionality - Introduced user permission grants for alerting actions in tests. - Added tests for rendering the More button with action menu options. - Verified that each rule has its own action buttons and handles permissions correctly. - Ensured the edit button is not rendered when user lacks edit permissions. - Confirmed the correct menu actions are displayed when the More button is clicked. * Update translations --------- Co-authored-by: Sonia Aguilar <[email protected]> Co-authored-by: Sonia Augilar <[email protected]>
|
cursor review |
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.
Bug: Silence Drawer Fails to Render
The silence drawer won't render when rule is undefined (list view scenario with only promRule available), even though the silence button can be shown and clicked based on grafanaPromRuleAbilities. The condition rulerRuleType.grafana.alertingRule(rule) returns false when rule is undefined, preventing the drawer from appearing despite showSilenceDrawer being true.
public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx#L103-L106
cursor-grafana/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx
Lines 103 to 106 in 3e6d620
| {deleteModal} | |
| {rulerRuleType.grafana.alertingRule(rule) && showSilenceDrawer && ( | |
| <SilenceGrafanaRuleDrawer rulerRule={rule} onClose={() => setShowSilenceDrawer(false)} /> | |
| )} |
Bug: Cannot Pause Prometheus Rules in Grafana
The pause menu item won't render when rulerRule is undefined (list view scenario), even though canPause can be true based on grafanaPauseAbility. The condition rulerRuleType.grafana.rule(rulerRule) returns false when rulerRule is undefined, preventing users from pausing rules when only promRule is available.
public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx#L116-L119
cursor-grafana/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx
Lines 116 to 119 in 3e6d620
| const shareUrl = createShareLink(identifier); | |
| const showDivider = | |
| [canPause, canSilence, shouldShowDeclareIncidentButton, canDuplicate].some(Boolean) && [canExport].some(Boolean); |
Bug: Conditional Delete Option Not Appearing
The delete menu item won't render when rulerRule is undefined (list view scenario), even though canDelete can be true based on grafanaDeleteAbility. The condition canDelete && rulerRule prevents the delete option from appearing when only promRule is available, and handleDelete requires a non-undefined rulerRule parameter.
public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx#L164-L173
cursor-grafana/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx
Lines 164 to 173 in 3e6d620
| )} | |
| {canDelete && rulerRule && ( | |
| <> | |
| <Menu.Divider /> | |
| <Menu.Item | |
| label={t('alerting.common.delete', 'Delete')} | |
| icon="trash-alt" | |
| destructive | |
| onClick={() => handleDelete(rulerRule, groupIdentifier)} | |
| /> |
| rule={rule} | ||
| groupIdentifier={groupIdentifier} | ||
| namespaceName={ruleWithOrigin.namespaceName} | ||
| showLocation={true} |
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.
Bug: Prevent React List Rendering Instability
The GrafanaRuleListItem component is missing the key prop in the list rendering. The key variable is extracted from ruleWithOrigin but not passed to the component, which prevents React from properly tracking list items and can cause rendering issues, lost component state, or performance problems.
Test 5
Replicated from ai-code-review-evaluation/grafana-cursor#5
Note
Refactors alerting rule list/menu to rely on Prometheus Grafana rules and new RBAC hooks, removing ruler dependencies and updating tests/utilities.
GrafanaRuleLoaderwithGrafanaRuleListItemand simplifyGrafanaGroupLoaderto fetch only Prometheus Grafana groups; remove ruler matching logic and loading states.AlertRuleMenuandRuleActionsButtons.V2to compute abilities from bothrulerRuleand Grafana Prom rule (promRule), enabling actions without ruler calls; construct editable identifiers frompromRule.useAllGrafanaPromRuleAbilities,useGrafanaPromRuleAbility/Abilities,skipToken; adduseIsGrafanaPromRuleEditableand silence/export checks.isProvisionedPromRuleandprovenancetoGrafanaPromRuleDTO.Written by Cursor Bugbot for commit 3e6d620. Configure here.