small fixes

This commit is contained in:
laurenashleigh
2026-01-12 12:34:31 +00:00
parent 60a4d15e71
commit d4e87cf510
3 changed files with 142 additions and 64 deletions
@@ -1,5 +1,5 @@
import { css } from '@emotion/css';
import { useEffect, useState } from 'react';
import { useEffect, useRef, useState } from 'react';
import { useFormContext } from 'react-hook-form';
import { GrafanaTheme2, ReducerID, SelectableValue, getNextRefId } from '@grafana/data';
@@ -30,7 +30,11 @@ const ExpressionDatasourceUID = '__expr__';
type LocalSimpleCondition = { whenField?: string; evaluator: { params: number[]; type: EvalFunction } };
// Helper function to create expression queries from simple condition
/**
* Creates expression queries (reduce + threshold) from a simple condition.
* These expression queries reference the last data query and build a pipeline:
* data query -> reduce expression -> threshold expression
*/
function createExpressionQueries(
simpleCondition: LocalSimpleCondition,
dataQueries: AlertQuery[]
@@ -80,6 +84,9 @@ function createExpressionQueries(
expression: reduceRefId,
};
// Expression queries don't need relativeTimeRange - they inherit time range context
// from the data queries they reference. This is consistent with how expressions work
// in the alerting query runner.
return {
reduce: {
refId: reduceRefId,
@@ -97,9 +104,29 @@ function createExpressionQueries(
};
}
/**
* Compares two AlertQuery arrays for expression-relevant equality.
* Only compares the model content of expression queries to determine
* if an update is actually needed.
*/
function areExpressionQueriesEqual(current: AlertQuery[], next: AlertQuery[]): boolean {
const currentExpressions = current.filter((q) => q.datasourceUid === ExpressionDatasourceUID);
const nextExpressions = next.filter((q) => q.datasourceUid === ExpressionDatasourceUID);
if (currentExpressions.length !== nextExpressions.length) {
return false;
}
try {
return JSON.stringify(currentExpressions) === JSON.stringify(nextExpressions);
} catch {
return false;
}
}
export function RuleConditionSection() {
const base = useStyles2(getStyles);
const { watch, setValue } = useFormContext<RuleFormValues>();
const { watch, setValue, getValues } = useFormContext<RuleFormValues>();
const evaluateFor = watch('evaluateFor') || '0s';
const queries = watch('queries');
watch('folder');
@@ -109,18 +136,39 @@ export function RuleConditionSection() {
evaluator: { params: [0], type: EvalFunction.IsAbove },
});
// Track if we're currently updating to prevent infinite loops
const isUpdatingRef = useRef(false);
// Update expression queries whenever simpleCondition changes
// We use a ref flag to prevent the infinite loop that would occur because:
// simpleCondition changes -> effect runs -> setValue updates queries -> queries change -> effect would run again
useEffect(() => {
const dataQueries = queries.filter((q) => q.datasourceUid !== ExpressionDatasourceUID);
// Skip if we're already in an update cycle
if (isUpdatingRef.current) {
return;
}
const currentQueries = getValues('queries');
const dataQueries = currentQueries.filter((q) => q.datasourceUid !== ExpressionDatasourceUID);
if (dataQueries.length === 0) {
return;
}
const { reduce, threshold, condition } = createExpressionQueries(simpleCondition, dataQueries);
const newQueries = [...dataQueries, reduce, threshold];
setValue('queries', [...dataQueries, reduce, threshold], { shouldDirty: false, shouldValidate: false });
setValue('condition', condition, { shouldDirty: false, shouldValidate: false });
}, [simpleCondition, queries, setValue]);
// Only update if the expression queries actually changed
if (!areExpressionQueriesEqual(currentQueries, newQueries)) {
isUpdatingRef.current = true;
setValue('queries', newQueries, { shouldDirty: false, shouldValidate: false });
setValue('condition', condition, { shouldDirty: false, shouldValidate: false });
// Reset the flag after the update cycle completes
requestAnimationFrame(() => {
isUpdatingRef.current = false;
});
}
}, [simpleCondition, getValues, setValue]);
const reducerOptions: Array<ComboboxOption<string>> = reducerTypes
.filter((o) => typeof o.value === 'string')
@@ -150,12 +198,12 @@ export function RuleConditionSection() {
};
return (
<div className={base.section}>
<section className={base.section} aria-labelledby="condition-section-heading">
<div className={base.sectionHeaderRow}>
<div className={base.sectionHeader}>
<Text element="h3" variant="h4" id="condition-section-heading">
{`2. `}
<Trans i18nKey="alerting.simplified.condition.title">Condition</Trans>
</div>
</Text>
</div>
<div>
@@ -189,6 +237,10 @@ export function RuleConditionSection() {
key={simpleCondition.evaluator.params[0]}
defaultValue={simpleCondition.evaluator.params[0] ?? ''}
onBlur={(event) => onEvaluateValueChange(event, 0)}
aria-label={t(
'alerting.simple-condition-editor.aria-label-threshold-from',
'Threshold from value'
)}
/>
<ToLabel />
<Input
@@ -197,6 +249,7 @@ export function RuleConditionSection() {
key={simpleCondition.evaluator.params[1]}
defaultValue={simpleCondition.evaluator.params[1] ?? ''}
onBlur={(event) => onEvaluateValueChange(event, 1)}
aria-label={t('alerting.simple-condition-editor.aria-label-threshold-to', 'Threshold to value')}
/>
</>
) : (
@@ -206,6 +259,7 @@ export function RuleConditionSection() {
key={simpleCondition.evaluator.params[0]}
defaultValue={simpleCondition.evaluator.params[0] ?? ''}
onBlur={(event) => onEvaluateValueChange(event, 0)}
aria-label={t('alerting.simple-condition-editor.aria-label-threshold', 'Threshold value')}
/>
)}
</Stack>
@@ -216,7 +270,7 @@ export function RuleConditionSection() {
{evaluateFor === '0s' && (
<Stack direction="row" gap={0.5} alignItems="center">
<Icon name="exclamation-triangle" />
<Icon name="exclamation-triangle" aria-hidden="true" />
<Text variant="bodySmall" color="secondary">
<Trans i18nKey="alerting.simplified.evaluation.immediate-warning">
Immediate firing might lead to unnecessary alerts being sent for temporary issues
@@ -226,7 +280,7 @@ export function RuleConditionSection() {
)}
</Stack>
</div>
</div>
</section>
);
}
@@ -239,12 +293,5 @@ function getStyles(theme: GrafanaTheme2) {
gap: theme.spacing(1),
marginBottom: theme.spacing(1),
}),
sectionHeader: css({
fontWeight: theme.typography.fontWeightRegular,
fontSize: theme.typography.h4.fontSize,
lineHeight: theme.typography.h4.lineHeight,
}),
paragraphRow: css({ display: 'flex', alignItems: 'center', flexWrap: 'wrap', gap: theme.spacing(1) }),
inlineField: css({ display: 'inline-flex' }),
};
}
@@ -5,7 +5,7 @@ import { useFormContext } from 'react-hook-form';
import { GrafanaTheme2 } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors';
import { Trans, t } from '@grafana/i18n';
import { Field, Input, Stack, useStyles2 } from '@grafana/ui';
import { Field, Input, Stack, Text, useStyles2 } from '@grafana/ui';
import { RuleFormType, RuleFormValues } from '../types/rule-form';
import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource';
@@ -30,21 +30,21 @@ export function RuleDefinitionSection({ type }: { type: RuleFormType }) {
const namePlaceholder = isRecording ? 'recording rule' : 'alert rule';
return (
<div className={styles.section}>
<section className={styles.section} aria-labelledby="rule-definition-section-heading">
<div className={styles.sectionHeaderRow}>
<div className={styles.sectionHeader}>
<Text element="h3" variant="h4" id="rule-definition-section-heading">
{`1. `}
<Trans i18nKey="alerting.simplified.rule-definition">Rule Definition</Trans>
</div>
</Text>
</div>
<div>
<Stack direction="column" gap={2}>
<Field
noMargin
label={
<span className={styles.nameLabel}>
<Text variant="bodySmall" weight="medium">
<Trans i18nKey="alerting.alert-rule-name-and-metric.label-name">Name</Trans>
</span>
</Text>
}
error={errors?.name?.message}
invalid={!!errors.name?.message}
@@ -99,7 +99,7 @@ export function RuleDefinitionSection({ type }: { type: RuleFormType }) {
)}
</Stack>
</div>
</div>
</section>
);
}
@@ -112,14 +112,5 @@ function getStyles(theme: GrafanaTheme2) {
gap: theme.spacing(1),
marginBottom: theme.spacing(1),
}),
sectionHeader: css({
fontWeight: theme.typography.fontWeightRegular,
fontSize: theme.typography.h4.fontSize,
lineHeight: theme.typography.h4.lineHeight,
}),
nameLabel: css({
fontSize: theme.typography.bodySmall.fontSize,
fontWeight: 500,
}),
};
}
@@ -20,6 +20,7 @@ import {
useStyles2,
} from '@grafana/ui';
import { useAppNotification } from 'app/core/copy/appNotification';
import { textUtil } from 'app/core/utils/text';
import { RuleFormValues } from '../types/rule-form';
import { Annotation } from '../utils/constants';
@@ -29,6 +30,51 @@ import { NeedHelpInfoForNotificationPolicy } from './rule-editor/NotificationsSt
// Centralized form path for selected contact point
const CONTACT_POINT_PATH = 'contactPoints.grafana.selectedContactPoint' as const;
/**
* Validates a URL string and returns an error message if invalid.
* Uses the URL constructor for parsing and rejects dangerous protocols.
* Returns undefined if the URL is valid or empty.
*/
function validateRunbookUrl(value: string): string | undefined {
const trimmedValue = value.trim();
// Empty values are allowed (field is optional)
if (!trimmedValue) {
return undefined;
}
try {
const url = new URL(trimmedValue);
// Reject dangerous URL schemes per F4 frontend security rule
if (url.protocol === 'javascript:' || url.protocol === 'data:' || url.protocol === 'vbscript:') {
return t(
'alerting.simplified.notification.runbook-url.invalid-protocol',
'Invalid URL protocol. Please use http or https.'
);
}
return undefined;
} catch {
return t(
'alerting.simplified.notification.runbook-url.invalid-format',
'Invalid URL format. Please enter a valid URL.'
);
}
}
/**
* Sanitizes a URL value before storing it in the form.
* Uses textUtil.sanitizeUrl to ensure safe URL handling.
*/
function sanitizeRunbookUrl(value: string): string {
const trimmedValue = value.trim();
if (!trimmedValue) {
return '';
}
// Use textUtil.sanitizeUrl for consistent URL sanitization across the codebase
return textUtil.sanitizeUrl(trimmedValue);
}
export function RuleNotificationSection() {
const styles = useStyles2(getStyles);
const notifyApp = useAppNotification();
@@ -78,15 +124,18 @@ export function RuleNotificationSection() {
const descriptionValue = getAnnotationValue(Annotation.description);
const runbookUrlValue = getAnnotationValue(Annotation.runbookURL);
// Validate runbook URL for form-level validation feedback
const runbookUrlError = useMemo(() => validateRunbookUrl(runbookUrlValue), [runbookUrlValue]);
const recipientLabelId = 'recipient-label';
return (
<div className={styles.section}>
<section className={styles.section} aria-labelledby="notification-section-heading">
<div className={styles.sectionHeaderRow}>
<div className={styles.sectionHeader}>
<Text element="h3" variant="h4" id="notification-section-heading">
{`3. `}
<Trans i18nKey="alerting.simplified.notification.title">Notification</Trans>
</div>
</Text>
</div>
<div>
@@ -229,7 +278,12 @@ export function RuleNotificationSection() {
/>
</Field>
<Field label={t('alerting.simplified.notification.runbook-url.label', 'Runbook URL (optional)')} noMargin>
<Field
label={t('alerting.simplified.notification.runbook-url.label', 'Runbook URL (optional)')}
noMargin
invalid={!!runbookUrlError}
error={runbookUrlError}
>
<Input
id="runbook-url-input"
type="url"
@@ -240,26 +294,15 @@ export function RuleNotificationSection() {
}}
onBlur={(e) => {
const value = e.currentTarget.value.trim();
// Validate URL on blur
if (value && value !== '') {
try {
const url = new URL(value);
// Reject dangerous URL schemes
if (url.protocol === 'javascript:' || url.protocol === 'data:' || url.protocol === 'vbscript:') {
notifyApp.error(
t(
'alerting.simplified.notification.runbook-url.invalid-protocol',
'Invalid URL protocol. Please use http or https.'
)
);
// Sanitize and update the URL on blur if it's valid
if (value) {
const error = validateRunbookUrl(value);
if (!error) {
// Sanitize the URL before storing
const sanitizedUrl = sanitizeRunbookUrl(value);
if (sanitizedUrl !== value) {
updateAnnotationValue(Annotation.runbookURL, sanitizedUrl);
}
} catch {
notifyApp.warning(
t(
'alerting.simplified.notification.runbook-url.invalid-format',
'Invalid URL format. Please enter a valid URL.'
)
);
}
}
}}
@@ -268,11 +311,13 @@ export function RuleNotificationSection() {
'Enter the webpage where you keep your runbook for the alert…'
)}
aria-label={t('alerting.simplified.notification.runbook-url.aria-label', 'Runbook URL')}
aria-invalid={!!runbookUrlError}
aria-describedby={runbookUrlError ? 'runbook-url-error' : undefined}
/>
</Field>
</Stack>
</div>
</div>
</section>
);
}
@@ -285,11 +330,6 @@ function getStyles(theme: GrafanaTheme2) {
gap: theme.spacing(1),
marginBottom: theme.spacing(1),
}),
sectionHeader: css({
fontWeight: theme.typography.fontWeightRegular,
fontSize: theme.typography.h4.fontSize,
lineHeight: theme.typography.h4.lineHeight,
}),
contentTopSpacer: css({ marginTop: theme.spacing(0.5) }),
manualRoutingInline: css({
display: 'inline-flex',