Automated Test: notification-rule-enhancements #327
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 <sonia.aguilar@grafana.com> * 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 <soniaaguilarpeiron@gmail.com> Co-authored-by: Sonia Augilar <sonia.aguilar@grafana.com>
📝 WalkthroughWalkthroughThis PR refactors the alerting unified rules infrastructure to consolidate permission/ability checks using multi-action hooks and expands support for Grafana-managed rules. Key changes include introducing useGrafanaPromRuleAbilities and useRulerRuleAbilities hooks, restructuring rule rendering components, removing redundant utilities, and adding Grafana rule provisioning detection. Changes
Sequence DiagramsequenceDiagram
participant Component as AlertRuleMenu<br/>Component
participant RulerHook as useRulerRuleAbilities<br/>Hook
participant GrafanaHook as useGrafanaPromRuleAbilities<br/>Hook
participant PermissionAPI as Permission<br/>System
participant RenderLogic as Render<br/>Logic
Component->>RulerHook: Pass rulerRule + actions
RulerHook->>PermissionAPI: Check Ruler permissions
PermissionAPI-->>RulerHook: Return ability[] for each action
RulerHook-->>Component: [ability1, ability2, ...]
Component->>GrafanaHook: Pass promRule + actions<br/>(or skipToken if absent)
GrafanaHook->>PermissionAPI: Check Grafana permissions
PermissionAPI-->>GrafanaHook: Return ability[] for each action
GrafanaHook-->>Component: [ability1, ability2, ...]
Component->>Component: Combine results with OR
Component->>RenderLogic: canPause, canDelete, canDuplicate<br/>(aggregated from both sources)
RenderLogic-->>Component: Rendered menu items based<br/>on combined permissions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.tsx (1)
24-28:⚠️ Potential issue | 🟡 MinorStale JSDoc: still references Ruler endpoint.
The comment says "Loads an evaluation group from Prometheus and Ruler endpoints" but the component no longer fetches from the Ruler endpoint.
Suggested fix
/** - * Loads an evaluation group from Prometheus and Ruler endpoints. + * Loads an evaluation group from the Prometheus endpoint. * Displays a loading skeleton while the data is being fetched. * Polls the Prometheus endpoint every 20 seconds to refresh the data. */public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx (1)
100-106:⚠️ Potential issue | 🟡 MinorSilence action is a no-op when
rulerRuleis absent.
handleSilence(Line 100) setsshowSilenceDrawertotrue, but theSilenceGrafanaRuleDraweron Line 104 only renders whenrulerRuleType.grafana.alertingRule(rule)is truthy. In the promRule-only scenario (list view), the silence menu item may be visible (viacanSilenceinAlertRuleMenu), but clicking it will do nothing — the drawer won't open and the state is silently set totruewith no visual feedback.Consider either:
- Passing a flag to
AlertRuleMenuto suppress the silence item whenrulerRuleis absent, or- Implementing a promRule-based silence path.
🤖 Fix all issues with AI agents
In `@public/app/features/alerting/unified/components/rules/RulesTable.test.tsx`:
- Around line 79-98: Remove the unused mock for useGrafanaPromRuleAbility in the
test: keep the mocks for useAlertRuleAbility (used by RuleActionsButtons) and
useGrafanaPromRuleAbilities (used by AlertRuleMenu) and delete the
mockImplementation block that references useGrafanaPromRuleAbility; ensure
RulesTable test still renders with the existing mocks and the assertion awaiting
ui.actionButtons.edit remains unchanged.
In `@public/app/features/alerting/unified/hooks/useAbilities.ts`:
- Around line 322-330: The Pause/Restore/ModifyExport/DeletePermanently
abilities are incorrectly gated by isAlertingRule; change those entries to use
the broader isGrafanaManagedAlertRule (the condition used by the deprecated
hook) so recording rules are included. Specifically, for
AlertRuleAction.ModifyExport, AlertRuleAction.Pause, AlertRuleAction.Restore,
and AlertRuleAction.DeletePermanently replace occurrences of isAlertingRule with
isGrafanaManagedAlertRule while keeping the existing surrounding checks (e.g.,
MaybeSupportedUnlessImmutable, isEditable, exportAllowed, and the isRemovable &&
isAdmin() expression) intact.
In `@public/app/features/alerting/unified/rule-list/FilterView.tsx`:
- Around line 155-163: The GrafanaRuleListItem JSX inside the switch is missing
the key prop used by its sibling branches; update the GrafanaRuleListItem
element to include the same key prop (key={key}) as used on DataSourceRuleLoader
and UnknownRuleListItem so that the mapped children have stable keys; locate the
GrafanaRuleListItem instantiation in the switch/case and add key={key} to its
props.
In `@public/app/features/alerting/unified/utils/rules.ts`:
- Around line 171-173: The function isProvisionedPromRule currently types its
parameter as PromRuleDTO but accesses provenance which only exists on
GrafanaPromRuleDTO; change the parameter type of isProvisionedPromRule(promRule:
PromRuleDTO) to isProvisionedPromRule(promRule: GrafanaPromRuleDTO) so the
provenance property is type-safe, leaving the body
(prometheusRuleType.grafana.rule(promRule) && Boolean(promRule.provenance))
unchanged.
🧹 Nitpick comments (7)
public/app/features/alerting/unified/hooks/useAbilities.ts (1)
287-289: Remove leftover// duplicatecomment.Line 289 has a
// duplicatecomment that appears to be a leftover debugging note.Suggested fix
- const { isEditable, isRemovable, loading } = useIsGrafanaPromRuleEditable(rule); // duplicate + const { isEditable, isRemovable, loading } = useIsGrafanaPromRuleEditable(rule);public/app/features/alerting/unified/rule-list/GrafanaRuleListItem.tsx (1)
41-42: Nit: unnecessary optional chaining on required prop.
ruleis a required prop in the interface, sorule?.healthandrule?.lastErrorcould berule.healthandrule.lastError. Harmless but slightly misleading.public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx (2)
56-100: Repetitive ability destructuring could be simplified with a helper.Each ability tuple is destructured and OR-combined identically across 5 actions. Consider extracting a small helper to reduce boilerplate, e.g.:
♻️ Suggested helper
function mergeAbilities( ruler: [boolean, boolean], grafana: [boolean, boolean] ): boolean { return (ruler[0] && ruler[1]) || (grafana[0] && grafana[1]); }Then:
const canPause = mergeAbilities(rulerPauseAbility, grafanaPauseAbility); const canDelete = mergeAbilities(rulerDeleteAbility, grafanaDeleteAbility); // ...etc
81-83:canPausederived from grafana prom abilities is unused whenrulerRuleis absent.
canPausecan betruevia the grafana prom abilities branch, but Line 123 additionally gates onrulerRuleType.grafana.rule(rulerRule), which isfalsewhenrulerRuleis undefined. So in the promRule-only scenario (list view), the pause menu item will never render despitecanPausebeingtrue. This appears intentional sinceMenuItemPauseRulerequires the ruler rule object, but it might confuse future readers. A brief inline comment clarifying this would help.Also applies to: 123-125
public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.test.tsx (1)
104-146: Consider asserting that the two rules' More buttons are distinct elements.The test checks each rule item has a More button and Edit button, which is good. A minor improvement would be asserting that the two More buttons are distinct DOM elements to guard against accidental shared state:
expect(ui.moreButton().get(ruleListItem1)).not.toBe(ui.moreButton().get(ruleListItem2));public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx (1)
118-128:getIsProvisionedfavorsruleoverpromRule— ensure this is intentional.If both
ruleandpromRuleare present but disagree on provisioned status (e.g., stale data), theruleresult always wins due to the early return on Line 119. This is likely fine sincerule(ruler rule) is the authoritative source, but worth noting.public/app/features/alerting/unified/components/rules/RulesTable.test.tsx (1)
20-31: Ensurejest.mockof the entire module doesn't break non-hook exports likeskipToken.
jest.mock('../../hooks/useAbilities')auto-mocks the entire module, replacing all exports withjest.fn()(for functions) orundefined(for non-function values). IfskipTokenis a constant (e.g., aSymbolor sentinel object), it will beundefinedin tests. The mocked hooks won't use it, but component code that referencesskipTokenin conditional expressions will evaluate it asundefined/falsy — which happens to work here since the mocks ignore their arguments. This is fragile if the implementation changes.Consider explicitly preserving non-hook exports:
jest.mock('../../hooks/useAbilities', () => ({ ...jest.requireActual('../../hooks/useAbilities'), useAlertRuleAbility: jest.fn(), useRulerRuleAbility: jest.fn(), useGrafanaPromRuleAbility: jest.fn(), useRulerRuleAbilities: jest.fn(), useGrafanaPromRuleAbilities: jest.fn(), }));
| it('Should not render Edit button for users without the update permission', async () => { | ||
| mocks.useRulerRuleAbility.mockImplementation((_rule, _groupIdentifier, action) => { | ||
| // Mock the specific hooks needed for Grafana rules | ||
| // Using singular hook for simpler mocking | ||
| mocks.useAlertRuleAbility.mockImplementation((rule, action) => { | ||
| return action === AlertRuleAction.Update ? [true, false] : [true, true]; | ||
| }); | ||
| mocks.useAlertRuleAbility.mockImplementation((_rule, action) => { | ||
| mocks.useGrafanaPromRuleAbility.mockImplementation((rule, action) => { | ||
| return action === AlertRuleAction.Update ? [true, false] : [true, true]; | ||
| }); | ||
| // Still need plural hook for AlertRuleMenu component | ||
| mocks.useGrafanaPromRuleAbilities.mockImplementation((rule, actions) => { | ||
| return actions.map((action) => { | ||
| return action === AlertRuleAction.Update ? [true, false] : [true, true]; | ||
| }); | ||
| }); | ||
|
|
||
| render(<RulesTable rules={[grafanaRule]} />); | ||
|
|
||
| await waitFor(() => expect(ui.actionButtons.edit.query()).not.toBeInTheDocument()); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find RulesTable component implementation
fd "RulesTable.tsx" --type=fRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 318
🏁 Script executed:
# Check which RuleActionsButtons variant is imported/used in RulesTable
rg -n "RuleActionsButtons" public/app/features/alerting/unified/components/rules/ --type=ts -C 3Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 14455
🏁 Script executed:
# Check what hooks are used in RuleActionsButtons variants
rg -n "useGrafanaPromRuleAbility\|useAlertRuleAbility" public/app/features/alerting/unified/components/rules/ --type=ts -B 2 -A 2Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 68
🏁 Script executed:
# Check what hooks are actually imported/used in RuleActionsButtons.tsx
cat -n public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx | head -100Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 4569
🏁 Script executed:
# Search for useGrafanaPromRuleAbility and useAlertRuleAbility in RuleActionsButtons
rg "useGrafanaPromRuleAbility|useAlertRuleAbility" public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsxRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 264
🏁 Script executed:
# Check what AlertRuleMenu uses
rg "useGrafanaPromRuleAbilities|useRulerRuleAbilities" public/app/features/alerting/unified/components/rules/ --type=ts -lRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 159
🏁 Script executed:
# Check AlertRuleMenu to see what hooks it actually uses
rg -n "useGrafanaPromRuleAbilities|useRulerRuleAbilities" public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx -A 2Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 572
🏁 Script executed:
# Let's also see what the test mocks are actually doing and if they're needed
cat -n public/app/features/alerting/unified/components/rules/RulesTable.test.tsx | sed -n '20,40p'Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 997
🏁 Script executed:
# Check if useGrafanaPromRuleAbility is used anywhere in the test file
rg -n "useGrafanaPromRuleAbility" public/app/features/alerting/unified/components/rules/RulesTable.test.tsxRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 422
Remove the unnecessary useGrafanaPromRuleAbility mock (line 85-87).
RuleActionsButtons only uses useAlertRuleAbility. AlertRuleMenu, which is rendered by RuleActionsButtons, uses the plural useGrafanaPromRuleAbilities, not the singular useGrafanaPromRuleAbility. The singular mock is unused and can be removed.
🤖 Prompt for AI Agents
In `@public/app/features/alerting/unified/components/rules/RulesTable.test.tsx`
around lines 79 - 98, Remove the unused mock for useGrafanaPromRuleAbility in
the test: keep the mocks for useAlertRuleAbility (used by RuleActionsButtons)
and useGrafanaPromRuleAbilities (used by AlertRuleMenu) and delete the
mockImplementation block that references useGrafanaPromRuleAbility; ensure
RulesTable test still renders with the existing mocks and the assertion awaiting
ui.actionButtons.edit remains unchanged.
| [AlertRuleAction.Explore]: toAbility(AlwaysSupported, AccessControlAction.DataSourcesExplore), | ||
| [AlertRuleAction.Silence]: canSilence, | ||
| [AlertRuleAction.ModifyExport]: [isGrafanaManagedAlertRule, exportAllowed], | ||
| [AlertRuleAction.Pause]: [MaybeSupportedUnlessImmutable && isGrafanaManagedAlertRule, isEditable ?? false], | ||
| [AlertRuleAction.Restore]: [MaybeSupportedUnlessImmutable && isGrafanaManagedAlertRule, isEditable ?? false], | ||
| [AlertRuleAction.Silence]: [silenceSupported, canSilenceInFolder && isAlertingRule], | ||
| [AlertRuleAction.ModifyExport]: [isAlertingRule, exportAllowed], | ||
| [AlertRuleAction.Pause]: [MaybeSupportedUnlessImmutable && isAlertingRule, isEditable ?? false], | ||
| [AlertRuleAction.Restore]: [MaybeSupportedUnlessImmutable && isAlertingRule, isEditable ?? false], | ||
| [AlertRuleAction.DeletePermanently]: [ | ||
| MaybeSupportedUnlessImmutable && isGrafanaManagedAlertRule, | ||
| MaybeSupportedUnlessImmutable && isAlertingRule, | ||
| (isRemovable && isAdmin()) ?? false, | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if recording rules support pausing in the codebase
rg -n --type=ts 'isPaused' -C2 -g '!*.test.*' -g '!*mock*' | head -100Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 7261
🏁 Script executed:
#!/bin/bash
# Find GrafanaPromRuleDTOBase definition
rg -n 'GrafanaPromRuleDTOBase|GrafanaAlertRule' --type=ts -A 10 -g '!*.test.*' -g '!*mock*' | head -80Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 6866
🏁 Script executed:
#!/bin/bash
# Find useAllRulerRuleAbilities implementation and how it handles Pause/Restore
rg -n 'useAllRulerRuleAbilities|isGrafanaManagedAlertRule' --type=ts -B 2 -A 15 -g '!*.test.*' -g '!*mock*' | head -150Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 10837
🏁 Script executed:
#!/bin/bash
# Get the full useAllGrafanaPromRuleAbilities implementation, particularly lines 320-340
sed -n '287,340p' public/app/features/alerting/unified/hooks/useAbilities.tsRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 3094
🏁 Script executed:
#!/bin/bash
# Search for recording rule pausing in tests or documentation
rg -n 'recording.*pause|pause.*recording' --type=ts -i -g '!node_modules' | head -20Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 3193
🏁 Script executed:
#!/bin/bash
# Look at the RuleViewer.test.tsx test to understand recording rule pausing better
sed -n '189,210p' public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsxRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 982
Recording rules should also support Pause/Restore/ModifyExport/DeletePermanently actions.
The new code at lines 322–330 restricts these actions to alerting rules only via isAlertingRule, but recording rules have the isPaused property and support pausing in Grafana (see GrafanaPromRecordingRuleDTO extending GrafanaPromRuleDTOBase, import UI with pauseRecordingRules option, and RuleViewer.test.tsx test case for paused recording rules). The old useAllRulerRuleAbilities correctly uses isGrafanaManagedAlertRule which includes both rule types. Update these actions to use a condition that includes recording rules, matching the behavior of the deprecated hook.
🤖 Prompt for AI Agents
In `@public/app/features/alerting/unified/hooks/useAbilities.ts` around lines 322
- 330, The Pause/Restore/ModifyExport/DeletePermanently abilities are
incorrectly gated by isAlertingRule; change those entries to use the broader
isGrafanaManagedAlertRule (the condition used by the deprecated hook) so
recording rules are included. Specifically, for AlertRuleAction.ModifyExport,
AlertRuleAction.Pause, AlertRuleAction.Restore, and
AlertRuleAction.DeletePermanently replace occurrences of isAlertingRule with
isGrafanaManagedAlertRule while keeping the existing surrounding checks (e.g.,
MaybeSupportedUnlessImmutable, isEditable, exportAllowed, and the isRemovable &&
isAdmin() expression) intact.
| case 'grafana': | ||
| return ( | ||
| <GrafanaRuleLoader | ||
| key={key} | ||
| ruleIdentifier={{ ruleSourceName: 'grafana', uid: rule.uid }} | ||
| <GrafanaRuleListItem | ||
| rule={rule} | ||
| groupIdentifier={groupIdentifier} | ||
| namespaceName={ruleWithOrigin.namespaceName} | ||
| showLocation={true} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Missing key prop on GrafanaRuleListItem.
Static analysis correctly flags that the GrafanaRuleListItem element inside the .map() is missing a key prop. Both sibling branches (DataSourceRuleLoader on line 165 and UnknownRuleListItem on line 168) pass key={key}, but this one does not.
Suggested fix
case 'grafana':
return (
<GrafanaRuleListItem
+ key={key}
rule={rule}
groupIdentifier={groupIdentifier}
namespaceName={ruleWithOrigin.namespaceName}
showLocation={true}
/>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case 'grafana': | |
| return ( | |
| <GrafanaRuleLoader | |
| key={key} | |
| ruleIdentifier={{ ruleSourceName: 'grafana', uid: rule.uid }} | |
| <GrafanaRuleListItem | |
| rule={rule} | |
| groupIdentifier={groupIdentifier} | |
| namespaceName={ruleWithOrigin.namespaceName} | |
| showLocation={true} | |
| /> | |
| ); | |
| case 'grafana': | |
| return ( | |
| <GrafanaRuleListItem | |
| key={key} | |
| rule={rule} | |
| groupIdentifier={groupIdentifier} | |
| namespaceName={ruleWithOrigin.namespaceName} | |
| showLocation={true} | |
| /> | |
| ); |
🧰 Tools
🪛 Biome (2.3.13)
[error] 157-163: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🤖 Prompt for AI Agents
In `@public/app/features/alerting/unified/rule-list/FilterView.tsx` around lines
155 - 163, The GrafanaRuleListItem JSX inside the switch is missing the key prop
used by its sibling branches; update the GrafanaRuleListItem element to include
the same key prop (key={key}) as used on DataSourceRuleLoader and
UnknownRuleListItem so that the mapped children have stable keys; locate the
GrafanaRuleListItem instantiation in the switch/case and add key={key} to its
props.
| export function isProvisionedPromRule(promRule: PromRuleDTO): boolean { | ||
| return prometheusRuleType.grafana.rule(promRule) && Boolean(promRule.provenance); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the file in question and surrounding context
cat -n public/app/features/alerting/unified/utils/rules.ts | head -180 | tail -30Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 1484
🏁 Script executed:
#!/bin/bash
# Find the PromRuleDTO definition
rg -n "type PromRuleDTO|interface PromRuleDTO" public/app/features/alerting/unifiedRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Find GrafanaPromRuleDTO definition
rg -n "type GrafanaPromRuleDTO|interface GrafanaPromRuleDTO" public/app/features/alerting/unifiedRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Check the DTO file for type definitions
find public/app/features/alerting -name "*dto*" -o -name "*types*" | head -10Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 427
🏁 Script executed:
#!/bin/bash
# Let's look for where PromRuleDTO and related types are defined
rg -n "PromRuleDTO|PromAlertingRuleDTO|PromRecordingRuleDTO" public/app/features/alerting/unified/utils/rules.ts | head -20Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 636
🏁 Script executed:
#!/bin/bash
# Search for all callers of isProvisionedPromRule
rg -n "isProvisionedPromRule" public/app/features/alerting --type=tsRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 765
🏁 Script executed:
#!/bin/bash
# Check the call site in RuleActionsButtons.V2.tsx
cat -n public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx | sed -n '115,135p'Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 625
🏁 Script executed:
#!/bin/bash
# Check the call site in useAbilities.ts
cat -n public/app/features/alerting/unified/hooks/useAbilities.ts | sed -n '285,305p'Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 1532
🏁 Script executed:
#!/bin/bash
# Find where PromRuleDTO is defined
rg -n "type PromRuleDTO\s*=" public/app/features/alerting/unified/typesRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Look for PromRuleDTO in a broader search
rg -n "PromRuleDTO" public/app/features/alerting/unified/types -A2Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Check if there are type definitions related to PromRule in the types directory
ls -la public/app/features/alerting/unified/types/Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 840
🏁 Script executed:
#!/bin/bash
# Check imports at top of rules.ts to find where PromRuleDTO comes from
head -50 public/app/features/alerting/unified/utils/rules.tsRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 1379
🏁 Script executed:
#!/bin/bash
# Search for where Rule type is defined
rg -n "export type Rule\s*=" public/app/features/alerting --type=ts | head -5Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Search for GrafanaPromRuleDTO definition
rg -n "GrafanaPromRuleDTO|GrafanaPromAlertingRuleDTO|GrafanaPromRecordingRuleDTO" public/app/features/alerting/unified --type=ts -B2 -A5 | head -100Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 9197
🏁 Script executed:
#!/bin/bash
# Find PromRuleDTO definition more broadly
rg -n "type PromRuleDTO|PromRuleDTO =" public/app --type=ts | head -20Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 336
🏁 Script executed:
#!/bin/bash
# Look at the type definitions in unified-alerting-dto.ts around the PromRuleDTO and related types
cat -n public/app/types/unified-alerting-dto.ts | sed -n '155,200p'Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 1730
🏁 Script executed:
#!/bin/bash
# Search for provenance field in the DTO file
rg -n "provenance" public/app/types/unified-alerting-dto.ts -B2 -A2Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 412
🏁 Script executed:
#!/bin/bash
# Look for PromAlertingRuleDTO and PromRecordingRuleDTO definitions
rg -n "type PromAlertingRuleDTO|type PromRecordingRuleDTO|interface PromAlertingRuleDTO|interface PromRecordingRuleDTO" public/app/types/unified-alerting-dto.ts -A5Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 569
🏁 Script executed:
#!/bin/bash
# Look for GrafanaPromAlertingRuleDTO and GrafanaPromRecordingRuleDTO definitions
rg -n "type GrafanaPromAlertingRuleDTO|type GrafanaPromRecordingRuleDTO|interface GrafanaPromAlertingRuleDTO|interface GrafanaPromRecordingRuleDTO" public/app/types/unified-alerting-dto.ts -A5Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 603
🏁 Script executed:
#!/bin/bash
# Check the PromRuleDTOBase definition to see if it has provenance
rg -n "interface PromRuleDTOBase" public/app/types/unified-alerting-dto.ts -A10Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 369
🏁 Script executed:
#!/bin/bash
# Check GrafanaPromRuleDTOBase definition
rg -n "interface GrafanaPromRuleDTOBase" public/app/types/unified-alerting-dto.ts -B2 -A10Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 420
🏁 Script executed:
#!/bin/bash
# Find Rule type definition
rg -n "export type Rule\s*=" public/app/types/unified-alerting.tsRepository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Check if Rule and PromRuleDTO are related
head -100 public/app/types/unified-alerting.ts | tail -50Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 1320
🏁 Script executed:
#!/bin/bash
# Let's see if the code actually compiles - check for TypeScript errors
# Look for tsconfig to understand TypeScript strictness
cat public/tsconfig.json 2>/dev/null | head -30 || echo "tsconfig not found in public folder"Repository: admin-coderabbit/coderabbit-grafana-mirror
Length of output: 68
Change parameter type to GrafanaPromRuleDTO for type safety.
The parameter promRule: PromRuleDTO type does not have a provenance field. PromRuleDTO is PromAlertingRuleDTO | PromRecordingRuleDTO, both of which extend PromRuleDTOBase which lacks provenance. Only GrafanaPromRuleDTOBase declares the provenance? field, and both callers already pass GrafanaPromRuleDTO types (or compatible types), so the parameter can be safely narrowed.
Suggested fix
-export function isProvisionedPromRule(promRule: PromRuleDTO): boolean {
- return prometheusRuleType.grafana.rule(promRule) && Boolean(promRule.provenance);
+export function isProvisionedPromRule(promRule: GrafanaPromRuleDTO): boolean {
+ return Boolean(promRule.provenance);
}🤖 Prompt for AI Agents
In `@public/app/features/alerting/unified/utils/rules.ts` around lines 171 - 173,
The function isProvisionedPromRule currently types its parameter as PromRuleDTO
but accesses provenance which only exists on GrafanaPromRuleDTO; change the
parameter type of isProvisionedPromRule(promRule: PromRuleDTO) to
isProvisionedPromRule(promRule: GrafanaPromRuleDTO) so the provenance property
is type-safe, leaving the body (prometheusRuleType.grafana.rule(promRule) &&
Boolean(promRule.provenance)) unchanged.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
Release Notes