feat(alert-rules): implement FormNG with sidebar navigation and CardC…#2144
Conversation
…ontainer pattern - Add FormNG directory with section-based alert rule form, sidebar, and scroll sync - Add CardContainer, FormItemLabel, RadioCard, SectionCard composable components - Migrate all plugin datasource AlertRule Queries to CardContainer pattern - Add cn() utility using clsx + tailwind-merge - Update i18n for all supported locales - Switch Add/Edit pages to import FormNG
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (112)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive FormNG rewrite of the alert rule creation/edit form (sidebar navigation, section cards, Prometheus v1/v2/Host/Loki rule editors, triggers subsystem, pipeline/workflow/relabel configuration, effective scheduling, notification config), migrates plugin query UIs to shared CardContainer components, updates 5 locales, and adds a full Playwright/Midscene config-driven E2E test suite covering all supported datasource categories, plus supporting AI-assistant documentation. ChangesFormNG Alert Rule Form
Config-Driven E2E Test Suite
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant FormNG
participant handleCheck
participant processFormValues
participant EditStrategy_addStrategy
User->>FormNG: Click Save
FormNG->>FormNG: form.validateFields()
FormNG->>handleCheck: pre-submit gating
handleCheck-->>FormNG: pass/fail
FormNG->>processFormValues: transform values
FormNG->>EditStrategy_addStrategy: submit to API
EditStrategy_addStrategy-->>FormNG: handleMessage(res)
FormNG->>User: navigate /alert-rules
sequenceDiagram
participant Test as index.test.ts
participant UI as FormNG Page
participant StepHandlers
participant API as Backend API
Test->>API: doLogin / fetchReferenceData
Test->>Test: normalizeAlertRuleForUi(config, refs)
Test->>UI: navigate to add alert rule
Test->>StepHandlers: fillBasicStep/fillDatasourceStep/fillRuleStep/...
StepHandlers->>UI: fill form fields
Test->>UI: click Save
UI->>API: POST alert rule
API-->>Test: save response
Test->>API: query created rule
Test->>Test: assert subset match
Test->>API: delete created rule
Related Issues: Not specified in the provided data. Related PRs: Not specified in the provided data. Suggested labels: enhancement, alert-rules, e2e-testing, i18n Suggested reviewers: Not specified — recommend maintainers familiar with alert rules FormNG and Playwright/Midscene E2E tooling. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/plugins/doris/AlertRule/Query.tsx (1)
29-40: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the FormNG
FormStateContexthere.This component now sits in the FormNG flow, but Line 32 still reads the legacy
FormStateContext. Because the two providers are different React contexts,typefalls back to the default value here, and Line 40 keepsshowDatabasefalse even for edit/clone rules that already have a database configured.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugins/doris/AlertRule/Query.tsx` around lines 29 - 40, The Query component is still reading the legacy form context, so it never sees the correct form type in the FormNG flow. Update the context lookup in Query to use the FormNG FormStateContext provider instead of the old one, and keep the existing showDatabase logic tied to that type value so edit/clone rules can correctly reveal the database field when a database is already present.
🟡 Minor comments (9)
src/pages/alertRules/FormNG/Rule/Log/utils.ts-1-15 (1)
1-15: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
'day'to theunitunion. The helper already handles'day', so the signature should match the implementation.Suggested fix
-export const normalizeTime = (value?: number, unit?: 'second' | 'min' | 'hour') => { +export const normalizeTime = (value?: number, unit?: 'second' | 'min' | 'hour' | 'day') => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/Rule/Log/utils.ts` around lines 1 - 15, The normalizeTime helper already has logic for a day unit, but its unit parameter type only allows second, min, and hour. Update the normalizeTime signature so the unit union includes day, keeping the type definition aligned with the existing branching logic in normalizeTime.src/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/index.tsx-82-84 (1)
82-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the stable
idasrowKey.
nameis editable and not guaranteed unique, so renamed or duplicate variables can make React reuse the wrong row. The rest of this component already treatsidas the stable identifier.Proposed change
- rowKey='name' + rowKey='id'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/index.tsx` around lines 82 - 84, The VariablesConfig table is using an editable, non-unique field as the row identifier, which can cause React to reuse the wrong row when variable names change or duplicate. Update the Table in the VariablesConfig component to use the stable id field as rowKey, matching the rest of the component’s identifier handling so rows stay consistent across edits.src/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/ChildVariablesConfigs/index.tsx-123-129 (1)
123-129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreview width is computed from row count, not column count.
childVarConfigs.param_valis the table data array, sogetColumnKeys(childVarConfigs.param_val).lengthtracks rows and usually resolvesnameasundefined. As rows are added, each preview gets narrower even though the number of columns is unchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/ChildVariablesConfigs/index.tsx` around lines 123 - 129, The preview width calculation in the `HostSelectPreview` and `NetworkDeviceSelectPreview` render path is using `getColumnKeys(childVarConfigs.param_val).length`, which is based on table rows instead of the column count. Update the width logic in `ChildVariablesConfigs` to derive from the actual number of columns/visible fields for the preview rather than `param_val`, so the `maxLength` stays stable as rows change and doesn’t shrink incorrectly.src/pages/alertRules/FormNG/components/SectionCard/index.tsx-14-18 (1)
14-18: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the hardcoded section colors with theme tokens.
text-red-900andbg-violet-200bypass the shared variable system, so these states can drift from the rest of the alert-rule UI in dark/light themes. As per coding guidelines,src/**/*.{ts,tsx,less,css}: Use color and theme-related values fromsrc/theme/variable.cssand existing theme system; avoid magic color values.Also applies to: 61-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/components/SectionCard/index.tsx` around lines 14 - 18, The section tag styles in tagClassesMap use hardcoded color utilities that bypass the shared theme variables. Update the core and any other affected tag class entries to use existing theme token classes from the alert-rule UI system instead of direct color values, keeping the mapping in SectionCard aligned with src/theme/variable.css and the shared theme conventions.Source: Coding guidelines
src/pages/alertRules/locale/ru_RU.ts-424-424 (1)
424-424: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the mixed-language Russian label.
Пороговое判定still contains the Chinese判定, so the threshold section will render broken copy.Suggested fix
- threshold_judgment: 'Пороговое判定', + threshold_judgment: 'Пороговая проверка',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/locale/ru_RU.ts` at line 424, The threshold judgment label in the ru_RU locale still mixes Russian and Chinese text; update the `threshold_judgment` entry to use a fully Russian translation. Fix the string in the `ru_RU` locale object so the alert rules threshold section renders consistent copy, and verify any related localization keys in the same file for similar mixed-language values.src/pages/alertRules/FormNG/components/Sidebar/RuleSummary.tsx-69-75 (1)
69-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize
SwitchFielddefaults instead of hardcoding Chinese text.
trueText = '是'/falseText = '否'leaks intoEffectiveSummary, so non-Chinese locales will still render Chinese in the sidebar summary. Pull these strings fromt(...)or require callers to pass them explicitly.Also applies to: 332-332
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/components/Sidebar/RuleSummary.tsx` around lines 69 - 75, SwitchField is hardcoding Chinese fallback labels, which makes the sidebar summary ignore the active locale. Update the SwitchField component in RuleSummary.tsx so the default trueText and falseText come from i18n via t(...) or are always passed in by callers, and then update EffectiveSummary and any other SwitchField usages to provide localized text explicitly where needed.src/pages/alertRules/FormNG/components/Sidebar/RuleSummary.tsx-255-255 (1)
255-255: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
weekdays_shortnever refreshes after a locale switch.This memo depends on translated output but has an empty dependency list, so the weekday labels stay in the previous language until remount. Depend on
t/i18n.language, or inline the lookup.Proposed fix
- const weekdays = useMemo(() => t('form_ng.weekdays_short', { returnObjects: true }) as string[], []); + const weekdays = useMemo(() => t('form_ng.weekdays_short', { returnObjects: true }) as string[], [t]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/components/Sidebar/RuleSummary.tsx` at line 255, The weekdays_short labels in RuleSummary are memoized with an empty dependency list, so they do not update after a locale change. Update the useMemo around the weekdays constant to depend on t and/or i18n.language (or remove the memo and inline the translation lookup) so the labels refresh when the language switches.src/pages/alertRules/FormNG/Notify/NotificationRuleSelect.tsx-26-44 (1)
26-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSet
loadingbefore starting the rule-list fetch.
fetchData()only clearsloadinginfinally, so the refresh icon never reflects in-flight work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/Notify/NotificationRuleSelect.tsx` around lines 26 - 44, The fetchData function in NotificationRuleSelect should set loading to true before calling getNotificationRules so the UI reflects the in-flight request; update the fetchData flow to start with setLoading(true), then keep the existing success, catch, and finally handling that clears loading. Use the existing fetchData and setLoading symbols to make the change local and consistent.src/pages/alertRules/FormNG/Notify/VersionSwitch.tsx-22-31 (1)
22-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReplace the bare anchor with a button.
<a>withouthrefisn’t keyboard-focusable here, so keyboard users can’t trigger the version switch. UseButton type='link'or abuttonwithtype='button'instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/Notify/VersionSwitch.tsx` around lines 22 - 31, The version switch in VersionSwitch should not use a bare anchor without an href, since it is not keyboard-accessible. Update the clickable element in the VersionSwitch component to use Button type="link" or a native button with type="button", and keep the existing onClick logic that calls onChange with the toggled value.
🧹 Nitpick comments (12)
src/pages/alertRules/FormNG/components/Triggers/AnomalyTrigger/services.ts (1)
4-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the return type with the actual consumer contract.
AnomalyTrigger/index.tsxreads this payload as a key/value map (_.keys,_.map(object)), but this helper advertises an array of maps. That mismatch hides invalid option/value handling at compile time; normalize the response to one shape and type it consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/components/Triggers/AnomalyTrigger/services.ts` around lines 4 - 9, The getAlgorithms helper returns a value shape that does not match how AnomalyTrigger/index.tsx consumes it, since the consumer treats the payload as a key/value object rather than an array of maps. Update getAlgorithms in services.ts and the related types so the return contract is a single consistent shape (prefer the object/map form used by _.keys and _.map(object)), and adjust the response normalization accordingly so invalid option/value handling is caught by the type system.src/pages/alertRules/FormNG/components/Triggers/Joins/index.tsx (1)
8-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anyin the props contract.
prefixField?: anyandqueries: any[]remove type checking for the nested field paths and query refs this component depends on. Please give both props explicit types so theForm.Listnames andSelectoptions stay safe to refactor.As per coding guidelines, "Component Props must use
interfacefor explicit declaration; avoidany".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/components/Triggers/Joins/index.tsx` around lines 8 - 13, The Props contract in Joins is using any for prefixField and queries, which disables type safety for the Form.List names and Select option references. Update the Props interface in the Joins component to use explicit types for these fields instead of any, keeping the nested path and query shapes precise and refactor-safe. Use the existing Props interface and the surrounding component usage to infer the correct concrete types, and ensure the public prop contract no longer contains any.Source: Coding guidelines
src/pages/alertRules/FormNG/Rule/index.tsx (1)
40-40: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the stale Loki props.
LokideclaresdatasourceValue: number[], but this router only passes the singulardatasource_valuewatcher, and the component doesn’t use either prop right now. Dropping the unused props or matching the signature will keep the contract consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/Rule/index.tsx` at line 40, The Loki usage in Rule/index.tsx is passing stale props that no longer match the component contract. Update the Router branch that renders Loki so it no longer forwards the unused datasourceCate/datasourceValue props, or else align Loki’s prop signature with the actual watcher shape if those values are still needed. Use the Loki component and the DatasourceCateEnum.loki branch as the reference points when making the change.src/pages/alertRules/FormNG/components/SectionCard/index.tsx (1)
36-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine
SectionCardprops with an interface.This shared component is the one new place in the cohort still using an inline props object. Please extract a
SectionCardPropsinterface so it matches the repo rule and stays easier to reuse. As per coding guidelines,src/**/*.{ts,tsx}: Component Props must useinterfacefor explicit declaration; avoidany.Suggested change
-export default function SectionCard(props: { +interface SectionCardProps { className?: string; item: SectionItem; index: number; sectionRef: (node: HTMLDivElement | null) => void; children?: React.ReactNode; empty?: boolean; collapsed?: boolean; setCollapsed?: (collapsed: boolean) => void; -}) { +} + +export default function SectionCard(props: SectionCardProps) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/components/SectionCard/index.tsx` around lines 36 - 45, The SectionCard component still defines its props inline, which violates the repo’s component props convention. Extract the props shape from the SectionCard default export into a named SectionCardProps interface, then use that interface in the function signature so the component remains easy to reuse and aligns with the existing rule.Source: Coding guidelines
src/pages/alertRules/FormNG/components/RadioCard/index.tsx (1)
23-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
cn()for this shared Tailwind wrapper.
classnameskeeps conflicting utilities, so consumerclassNameprops cannot reliably override defaults on this primitive. Switching to the new shared merge helper makes extensions predictable.Suggested change
-import classnames from 'classnames'; +import { cn } from '`@/utils`'; ... - className={classnames( + className={cn( 'items-start w-full min-h-[50px] mr-0 px-4 py-2.5 border border-solid border-[var(--fc-border-color)] rounded-lg bg-[var(--fc-fill-1)] transition-colors', '[&_.ant-radio]:top-0.5 [&_.ant-radio]:shrink-0', 'hover:border-[var(--fc-violet-7)]', '[&.ant-radio-wrapper-checked]:border-[var(--fc-violet-6)] [&.ant-radio-wrapper-checked]:bg-[var(--fc-violet-2)] [&.ant-radio-wrapper-checked]:shadow-[0_0_0_1px_var(--fc-violet-5)]', '[&.ant-radio-wrapper-disabled]:cursor-not-allowed [&.ant-radio-wrapper-disabled]:bg-[var(--fc-fill-2)] [&.ant-radio-wrapper-disabled]:opacity-70', className, )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/components/RadioCard/index.tsx` around lines 23 - 30, The shared RadioCard wrapper in the RadioCard component is using classnames, which prevents later consumer Tailwind classes from reliably overriding the built-in defaults. Replace the class merging in the RadioCard component with the shared cn() helper so wrapper styles and the className prop are merged predictably, and keep the same existing styling tokens and checked/disabled variants intact.src/plugins/clickHouse/AlertRule/index.tsx (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the component props explicitly.
datasourceValueis currently an implicitany. Give this wrapper a small props interface before handing the value intoQueriesandTriggers. As per coding guidelines,src/**/*.{ts,tsx}: Component Props must useinterfacefor explicit declaration; avoidany.Suggested change
+interface Props { + datasourceValue: number | number[]; +} + -export default function index({ datasourceValue }) { +export default function index({ datasourceValue }: Props) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugins/clickHouse/AlertRule/index.tsx` at line 7, The wrapper component in index currently takes an implicit any for datasourceValue, so define an explicit props interface for this component and use it in the function signature. Update the index component to accept typed props, then pass the typed datasourceValue through to Queries and Triggers without relying on any; keep the prop declaration as an interface to match the project’s TypeScript component conventions.Source: Coding guidelines
src/plugins/TDengine/AlertRule/index.tsx (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a
Propsinterface for this component.Line 7 destructures an untyped props object directly, which breaks the repo's props-typing convention and can become an implicit-
anyproblem under stricter TypeScript settings.As per coding guidelines, "Component Props must use
interfacefor explicit declaration; avoidany."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugins/TDengine/AlertRule/index.tsx` at line 7, The default export component currently destructures an untyped props object, so add an explicit Props interface for this component and use it in the index function signature. Define the interface near index and include datasourceValue with the correct type instead of relying on implicit any, following the repo’s props-typing convention.Source: Coding guidelines
src/plugins/elasticsearch/AlertRule/index.tsx (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the inline props type to an interface.
Line 7 still types the component props inline instead of through an explicit interface, which drifts from the repo convention for
.tsxcomponent APIs.Suggested fix
+interface Props { + hideIndexPattern?: boolean; + datasourceValue: number; + disabled?: boolean; +} + -export default function index({ hideIndexPattern, datasourceValue, disabled }: { hideIndexPattern?: boolean; datasourceValue: number; disabled?: boolean }) { +export default function index({ hideIndexPattern, datasourceValue, disabled }: Props) {As per coding guidelines, "Component Props must use
interfacefor explicit declaration; avoidany."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugins/elasticsearch/AlertRule/index.tsx` at line 7, The AlertRule component in index still declares its props inline on the default export; extract those props into an explicit interface and use that interface in the function signature instead. Update the index component’s props declaration to follow the repo convention for .tsx component APIs, keeping the same fields (hideIndexPattern, datasourceValue, disabled) but moving the type definition out of the parameter list.Source: Coding guidelines
src/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/index.tsx (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the component to match the module.
Exporting
PrometheusV2from the relabel entry makes React DevTools and stack traces unnecessarily confusing in this area.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/index.tsx` at line 14, The default export in the relabel entry is still named PrometheusV2, which makes debugging and React DevTools confusing. Rename the component/function to match the relabel module’s purpose, and keep the default export aligned with that new name in the PrometheusV2 declaration so stack traces and component labels are clear.src/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/style.less (1)
1-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer Tailwind for these local layout rules.
This stylesheet only defines local flex/spacing/positioning that can be expressed directly in TSX, while the repo guidelines reserve Less for third-party overrides, reusable tokens, and animations. As per coding guidelines,
src/**/*.less: "Use Less/CSS for overriding third-party component internal structures... global/cross-page reusable tokens, keyframes, and animations" andsrc/**/*.{ts,tsx,less,css}: "Use Tailwind utility classes for container layout".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/style.less` around lines 1 - 18, The local layout styles in the relabel form should be moved out of the Less file and expressed with Tailwind utility classes directly in the TSX for the relevant relabel list/item wrappers and actions. Update the components that use .n9e-alert-relabel-list, .n9e-alert-relabel-item, and .n9e-alert-relabel-item-actions to apply equivalent flex, spacing, positioning, background, padding, and border-radius utilities, then remove these rules from the stylesheet so Less stays limited to reusable tokens or overrides.Source: Coding guidelines
src/pages/alertRules/FormNG/PipelineConfigsNG/Processor.tsx (1)
22-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the
addcallback type.
add(defaultValue?: any, ...)drops the processor shape exactly where this component duplicates form items. That makes future processor config changes easy to break silently. Use the concrete processor item type here, or at leastunknownplus a narrow cast at the callsite. As per coding guidelines, "Component Props must useinterfacefor explicit declaration; avoidany".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/PipelineConfigsNG/Processor.tsx` around lines 22 - 30, The Props interface in Processor should not use an any-typed add callback for duplicated form items. Update the add signature to use the concrete processor item type (or unknown with a narrow cast at the callsite in Processor) so the component stays aligned with the processor config shape and avoids silent breakage. Keep the change localized to the Props definition and the add usage within Processor.Source: Coding guidelines
src/pages/alertRules/FormNG/Notify/TaskTpls/TplSettings.tsx (1)
8-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anyin this props contract.Both
fieldandtplsare core inputs for the template row, so typing them asanyremoves most of the safety this new form code should get from TypeScript. As per coding guidelines, "Component Props must useinterfacefor explicit declaration; avoidany".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertRules/FormNG/Notify/TaskTpls/TplSettings.tsx` around lines 8 - 10, The Props contract in TplSettings currently uses any for both field and tpls, which removes type safety from this component. Replace the loose typings in the TplSettings Props interface with explicit types that describe the template row inputs, and keep the props definition as an interface so the component can rely on TypeScript checks. Use the TplSettings and Props symbols to update the component’s prop contract without introducing any.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5fefdb3c-3c62-45ee-9510-e539849ed7ed
📒 Files selected for processing (80)
src/pages/alertRules/Add.tsxsrc/pages/alertRules/Edit.tsxsrc/pages/alertRules/FormNG/Effective/index.tsxsrc/pages/alertRules/FormNG/Notify/NotificationRuleSelect.tsxsrc/pages/alertRules/FormNG/Notify/TaskTpls/TplSettings.tsxsrc/pages/alertRules/FormNG/Notify/TaskTpls/index.tsxsrc/pages/alertRules/FormNG/Notify/VersionSwitch.tsxsrc/pages/alertRules/FormNG/Notify/index.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/Annotations.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/EnrichQueries.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/Processor.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/EventsModal.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/RelabelItem.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/TestModal.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/index.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/services.tssrc/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/style.lesssrc/pages/alertRules/FormNG/PipelineConfigsNG/WorkflowItem.tsxsrc/pages/alertRules/FormNG/PipelineConfigsNG/index.tsxsrc/pages/alertRules/FormNG/Rule/Host/Preview.tsxsrc/pages/alertRules/FormNG/Rule/Host/ValuesSelect.tsxsrc/pages/alertRules/FormNG/Rule/Host/index.tsxsrc/pages/alertRules/FormNG/Rule/Host/style.lesssrc/pages/alertRules/FormNG/Rule/Log/AdvancedSettings.tsxsrc/pages/alertRules/FormNG/Rule/Log/Loki/index.tsxsrc/pages/alertRules/FormNG/Rule/Log/utils.tssrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/GraphPreview.tsxsrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/PrometheusV2.tsxsrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/ChildVariablesConfigs/index.tsxsrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/EditModal.tsxsrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/index.tsxsrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/utils/syncChildVariables.tssrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/components/AdvancedSettings/index.tsxsrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/index.tsxsrc/pages/alertRules/FormNG/Rule/Metric/Prometheus/style.lesssrc/pages/alertRules/FormNG/Rule/index.tsxsrc/pages/alertRules/FormNG/components/CardContainer/index.tsxsrc/pages/alertRules/FormNG/components/DatasourceValueSelect/index.tsxsrc/pages/alertRules/FormNG/components/DatasourceValueSelect/services.tssrc/pages/alertRules/FormNG/components/DatasourceValueSelect/style.lesssrc/pages/alertRules/FormNG/components/FormItemLabel/index.tsxsrc/pages/alertRules/FormNG/components/RadioCard/index.tsxsrc/pages/alertRules/FormNG/components/SectionCard/index.tsxsrc/pages/alertRules/FormNG/components/Sidebar/RuleSummary.tsxsrc/pages/alertRules/FormNG/components/Sidebar/index.tsxsrc/pages/alertRules/FormNG/components/Triggers/AnomalyTrigger/AbnormalDetection/index.tsxsrc/pages/alertRules/FormNG/components/Triggers/AnomalyTrigger/index.tsxsrc/pages/alertRules/FormNG/components/Triggers/AnomalyTrigger/services.tssrc/pages/alertRules/FormNG/components/Triggers/Builder.tsxsrc/pages/alertRules/FormNG/components/Triggers/Code.tsxsrc/pages/alertRules/FormNG/components/Triggers/Joins/index.tsxsrc/pages/alertRules/FormNG/components/Triggers/NodataTrigger.tsxsrc/pages/alertRules/FormNG/components/Triggers/RecoverConfig/index.tsxsrc/pages/alertRules/FormNG/components/Triggers/Trigger.tsxsrc/pages/alertRules/FormNG/components/Triggers/Triggers.tsxsrc/pages/alertRules/FormNG/components/Triggers/index.tsxsrc/pages/alertRules/FormNG/index.tsxsrc/pages/alertRules/FormNG/utils/shouldShowAdvancedSettings.tssrc/pages/alertRules/FormNG/utils/useScrollSync.tssrc/pages/alertRules/locale/en_US.tssrc/pages/alertRules/locale/ja_JP.tssrc/pages/alertRules/locale/ru_RU.tssrc/pages/alertRules/locale/zh_CN.tssrc/pages/alertRules/locale/zh_HK.tssrc/plugins/TDengine/AlertRule/Queries/index.tsxsrc/plugins/TDengine/AlertRule/index.tsxsrc/plugins/clickHouse/AlertRule/Queries/index.tsxsrc/plugins/clickHouse/AlertRule/index.tsxsrc/plugins/doris/AlertRule/Query.tsxsrc/plugins/doris/AlertRule/index.tsxsrc/plugins/elasticsearch/AlertRule/Queries/Query.tsxsrc/plugins/elasticsearch/AlertRule/Queries/index.tsxsrc/plugins/elasticsearch/AlertRule/index.tsxsrc/plugins/mysql/AlertRule/Queries/index.tsxsrc/plugins/mysql/AlertRule/index.tsxsrc/plugins/pgsql/AlertRule/Queries/index.tsxsrc/plugins/pgsql/AlertRule/index.tsxsrc/plugins/victorialogs/AlertRule/Queries/index.tsxsrc/plugins/victorialogs/AlertRule/index.tsxsrc/utils/index.ts
… unify card layout with CardContainer - Create FormNGDataProvider to manage shared data (notification rules, teams, workflows, service calendars, webhooks, callbacks) with permission-gated loading - Migrate components (Notify, Effective, WorkflowItem, RuleSummary) to context - Replace manual card markup with CardContainer/CardContainerHeader in plugin EnrichQueries (doris, elasticsearch) - Remove unused imports and simplify local state management - Fix missing React key prop in SideMenu
Add config-driven E2E tests for alert rule creation covering Prometheus (v1/v2), Loki, and Elasticsearch datasources. Includes JSON configs, normalizer, query handlers, and Midscene AI-assisted form filling.
…with 3 new configs - Decompose monolithic test into step-based handlers: basic, datasource, rule, pipeline, effective, notify - Add config-driven test configs: base (prometheus), es-index-pattern, aliyun-sls - Mirror FormNG's processInitialValues/processFormValues for accurate persistence assertions - Add aliyun-sls query handler and enhance elasticsearch with index pattern support - Expand reference data: index patterns, pipelines, service calendars, datasource reverse map - Add Playwright config with configurable workers and parallel test mode - Add data-testid attributes to ES index pattern selectors for stable E2E locators
…rm validation - Add exp_trigger_disable switch to hide/disable threshold triggers section - Add validateDisabled prop flowing through Trigger -> Builder/Code/Severity - Conditionally render anomaly trigger and nodata trigger settings based on their respective enable switches - Disable form validation rules when triggers are disabled
Add two toolbar buttons to the alert rule form: "Core steps only" collapses optional sections, and "Expand all"/"Collapse all" toggles all visible sections at once.
…eanup - Set all Switch components to size='small' for consistent compact UI - Replace QuestionCircleFilled with QuestionCircleOutlined - Remove unused useMemo and Card imports - Restructure notify_recovered from Form.Item label to div+span - Move Switch outside span wrapper in WorkflowItem for proper spacing - Reformat long JSX lines for readability
Reduce duplication across aliyun-sls, ck, and tencent-cls query handlers by extracting common relative time range picker and advanced settings fill logic into shared helpers. Remove dead code, unused imports, and simplify generic type signatures.
- Wrap annotations in CardContainer for consistent styling - Replace relabel custom Less styles with Tailwind classes - Conditionally show relabel section only when historic configs exist - Add deprecation warning alert for event relabel with i18n entries - Sort datasource types in predefined order in dropdown - Display processor index as column letter (A, B, C...) - Fix locale indentation consistency in relabel object
…hPreview Add showSearch and optionFilterProp='label' to datasource Select in all GraphPreview components. Refactor PromGraphCpt Table.tsx controls layout to use InputGroupWithFormItem.
…ES plugins Remove explicit labelWidth props from InputGroupWithFormItem to let label width default to max-content, consistent with other datasource plugins.
…em across datasource plugins
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “FormNG” alert rule editor built around section cards + sidebar navigation, and migrates existing datasource-specific AlertRule query UIs to a shared CardContainer-based layout. It also adds a small className utility (cn) and introduces a Playwright + Midscene E2E harness for alert-rule creation flows.
Changes:
- Add FormNG alert rule form structure (sections, sidebar, composable cards/components) and switch Add/Edit pages to use it.
- Refactor multiple datasource AlertRule query editors to CardContainer pattern and improve datasource Select search UX.
- Add Playwright E2E setup + fixtures/helpers/configs for add-alert-rule scenarios.
Reviewed changes
Copilot reviewed 162 out of 164 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/index.ts | Add cn() utility (clsx + twMerge) |
| src/plugins/victorialogs/AlertRule/Queries/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/victorialogs/AlertRule/index.tsx | Switch to FormNG Triggers + spacing |
| src/plugins/TDengine/components/AdvancedSettings.tsx | Remove labelWidth overrides |
| src/plugins/TDengine/AlertRule/Queries/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/TDengine/AlertRule/index.tsx | Switch to FormNG Triggers + spacing |
| src/plugins/pgsql/AlertRule/Queries/index.tsx | Migrate queries UI to CardContainer pattern |
| src/plugins/pgsql/AlertRule/Queries/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/pgsql/AlertRule/index.tsx | Switch to FormNG Triggers + spacing |
| src/plugins/mysql/AlertRule/Queries/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/mysql/AlertRule/index.tsx | Switch to FormNG Triggers + spacing |
| src/plugins/elasticsearch/AlertRule/Queries/IndexPatternSelect.tsx | Add testid for E2E targeting |
| src/plugins/elasticsearch/AlertRule/Queries/index.tsx | Migrate queries UI + add button |
| src/plugins/elasticsearch/AlertRule/Queries/DateField.tsx | Remove labelWidth override |
| src/plugins/elasticsearch/AlertRule/index.tsx | Switch to FormNG Triggers + spacing |
| src/plugins/elasticsearch/AlertRule/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/elasticsearch/AlertRule/EnrichQueries/index.tsx | Use CardContainer + spacing adjustments |
| src/plugins/elasticsearch/AlertRule/EnrichQueries/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/doris/AlertRule/index.tsx | Migrate queries header/actions to FormNG style |
| src/plugins/doris/AlertRule/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/clickHouse/AlertRule/Queries/index.tsx | Migrate queries UI to CardContainer pattern |
| src/plugins/clickHouse/AlertRule/Queries/GraphPreview.tsx | Datasource Select search/filter tweaks |
| src/plugins/clickHouse/AlertRule/index.tsx | Switch to FormNG Triggers + spacing |
| src/pages/alertRules/FormNG/utils/shouldShowAdvancedSettings.ts | Add advanced-settings visibility helper |
| src/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/utils/syncChildVariables.ts | Sync child variables utility |
| src/pages/alertRules/FormNG/Rule/Metric/Prometheus/VariablesConfig/EditModal.tsx | Variable edit modal for Prometheus |
| src/pages/alertRules/FormNG/Rule/Metric/Prometheus/style.less | Prometheus rule styling |
| src/pages/alertRules/FormNG/Rule/Metric/Prometheus/GraphPreview.tsx | FormNG Prometheus preview popover |
| src/pages/alertRules/FormNG/Rule/Metric/Prometheus/components/AdvancedSettings/index.tsx | FormNG Prometheus advanced settings |
| src/pages/alertRules/FormNG/Rule/Log/utils.ts | Log rule time normalization utility |
| src/pages/alertRules/FormNG/Rule/Log/Loki/index.tsx | FormNG Loki query UI |
| src/pages/alertRules/FormNG/Rule/Log/AdvancedSettings.tsx | Log advanced settings toggle |
| src/pages/alertRules/FormNG/Rule/index.tsx | Route to datasource-specific FormNG rules |
| src/pages/alertRules/FormNG/Rule/Host/ValuesSelect.tsx | Host rule value selector + fetching |
| src/pages/alertRules/FormNG/Rule/Host/style.less | Host rule styling |
| src/pages/alertRules/FormNG/Rule/Host/Preview.tsx | Host query preview modal |
| src/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/TestModal.tsx | Relabel test modal (FormNG pipeline) |
| src/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/services.ts | Relabel test API call |
| src/pages/alertRules/FormNG/PipelineConfigsNG/Relabel/index.tsx | Relabel list UI (FormNG pipeline) |
| src/pages/alertRules/FormNG/PipelineConfigsNG/EnrichQueries.tsx | FormNG pipeline enrich queries switchboard |
| src/pages/alertRules/FormNG/PipelineConfigsNG/Annotations.tsx | FormNG annotations list UI |
| src/pages/alertRules/FormNG/Notify/VersionSwitch.tsx | Notify version switch control |
| src/pages/alertRules/FormNG/Notify/TaskTpls/TplSettings.tsx | Task template settings UI |
| src/pages/alertRules/FormNG/Notify/TaskTpls/index.tsx | Task templates list + fetch |
| src/pages/alertRules/FormNG/Notify/NotificationRuleSelect.tsx | Notification rule selector + drawer viewer |
| src/pages/alertRules/FormNG/components/Triggers/Trigger.tsx | Trigger card container + mode switch |
| src/pages/alertRules/FormNG/components/Triggers/RecoverConfig/index.tsx | Trigger recovery config UI |
| src/pages/alertRules/FormNG/components/Triggers/NodataTrigger.tsx | No-data trigger UI |
| src/pages/alertRules/FormNG/components/Triggers/index.tsx | Default-active trigger tab logic |
| src/pages/alertRules/FormNG/components/Triggers/Code.tsx | Trigger code-mode editor + validation |
| src/pages/alertRules/FormNG/components/Triggers/Builder.tsx | Trigger builder-mode expressions UI |
| src/pages/alertRules/FormNG/components/Triggers/AnomalyTrigger/services.ts | Fetch anomaly algorithms |
| src/pages/alertRules/FormNG/components/Triggers/AnomalyTrigger/index.tsx | Anomaly trigger UI |
| src/pages/alertRules/FormNG/components/Sidebar/index.tsx | Sidebar step navigation + summary panel |
| src/pages/alertRules/FormNG/components/SectionCard/index.tsx | SectionCard UI + collapse behavior |
| src/pages/alertRules/FormNG/components/RadioCard/index.tsx | Radio card component styling |
| src/pages/alertRules/FormNG/components/FormItemLabel/index.tsx | Shared form label component |
| src/pages/alertRules/FormNG/components/DatasourceValueSelect/style.less | Datasource select styling |
| src/pages/alertRules/FormNG/components/DatasourceValueSelect/services.ts | Datasource query API helper |
| src/pages/alertRules/FormNG/components/CardContainer/index.tsx | CardContainer layout + close affordance |
| src/pages/alertRules/Form/Rule/Rule/Metric/Prometheus/GraphPreview.tsx | Add Select search/filter in legacy preview |
| src/pages/alertRules/Form/components/Severity/index.tsx | Add validateDisabled option for severity |
| src/pages/alertRules/Edit.tsx | Switch Edit page to FormNG |
| src/pages/alertRules/Add.tsx | Switch Add page to FormNG + spacing |
| src/components/SideMenu/MenuList.tsx | Fix missing React key on group node |
| src/components/PromGraphCpt/Table.tsx | Refactor controls layout (InputGroupWithFormItem) |
| src/components/InputGroupWithFormItem/style.less | Adjust timerange picker border radius |
| playwright.config.ts | Add Playwright workers config |
| package.json | Add Playwright/Midscene deps + e2e script |
| e2e/types.ts | Shared Midscene fixture types |
| e2e/README.md | E2E documentation + env vars |
| e2e/fixture.ts | Playwright+Midscene fixture + login helpers |
| e2e/config-loader.ts | Load JSON configs with whitelist filtering |
| e2e/add-alert-rule/types.ts | Alert rule config + normalized UI types |
| e2e/add-alert-rule/steps/rule.ts | E2E step: rule/condition filling |
| e2e/add-alert-rule/steps/pipeline.ts | E2E step: pipeline + relabel + annotations |
| e2e/add-alert-rule/steps/notify.ts | E2E step: notify configuration |
| e2e/add-alert-rule/steps/datasource.ts | E2E step: datasource selection/filtering |
| e2e/add-alert-rule/steps/basic.ts | E2E step: basic form fields |
| e2e/add-alert-rule/queries/victorialogs.ts | E2E handler: victorialogs condition UI |
| e2e/add-alert-rule/queries/tdengine.ts | E2E handler: tdengine condition UI |
| e2e/add-alert-rule/queries/pgsql.ts | E2E handler: pgsql condition UI |
| e2e/add-alert-rule/queries/oracle.ts | E2E handler: oracle condition UI |
| e2e/add-alert-rule/queries/mysql.ts | E2E handler: mysql condition UI |
| e2e/add-alert-rule/queries/loki.ts | E2E handler: loki condition UI |
| e2e/add-alert-rule/queries/influxdb.ts | E2E handler: influxdb condition UI |
| e2e/add-alert-rule/queries/index.ts | Register condition handlers by cate |
| e2e/add-alert-rule/queries/doris.ts | E2E handler: doris condition UI |
| e2e/add-alert-rule/queries/ck.ts | E2E handler: clickhouse condition UI |
| e2e/add-alert-rule/helpers/range.ts | Helper: relative time range picker |
| e2e/add-alert-rule/helpers/index.ts | Helper exports + interval/duration fill |
| e2e/add-alert-rule/helpers/advanced-settings.ts | Helper: fill advanced settings controls |
| e2e/add-alert-rule/configs/vlogs.json | E2E config: victorialogs |
| e2e/add-alert-rule/configs/tls.json | E2E config: volc-tls |
| e2e/add-alert-rule/configs/tdengine.json | E2E config: tdengine |
| e2e/add-alert-rule/configs/sls.json | E2E config: aliyun-sls |
| e2e/add-alert-rule/configs/prometheus-v2.json | E2E config: prometheus v2 |
| e2e/add-alert-rule/configs/prometheus-v1.json | E2E config: prometheus v1 |
| e2e/add-alert-rule/configs/pgsql.json | E2E config: pgsql |
| e2e/add-alert-rule/configs/oracle.json | E2E config: oracle |
| e2e/add-alert-rule/configs/mysql.json | E2E config: mysql |
| e2e/add-alert-rule/configs/loki.json | E2E config: loki |
| e2e/add-alert-rule/configs/influxDB.json | E2E config: influxdb |
| e2e/add-alert-rule/configs/gcm.json | E2E config: gcm |
| e2e/add-alert-rule/configs/es-index.json | E2E config: elasticsearch index |
| e2e/add-alert-rule/configs/es-index-pattern.json | E2E config: elasticsearch index pattern |
| e2e/add-alert-rule/configs/doris.json | E2E config: doris |
| e2e/add-alert-rule/configs/cls.json | E2E config: tencent-cls |
| e2e/add-alert-rule/configs/cloudwatch.json | E2E config: cloudwatch |
| e2e/add-alert-rule/configs/ck.json | E2E config: clickhouse |
| e2e/add-alert-rule/configs/bls.json | E2E config: bce-bls |
| e2e/add-alert-rule/configs/base.json | E2E config: base scenario |
| .gitignore | Ignore Playwright test-results output |
| </Form.Item> | ||
| </div> | ||
| {field.name !== fields.length - 1 && ( | ||
| <Form.Item {...field} name={[0, 'logicalOperator']}> |
| @@ -0,0 +1,18 @@ | |||
| export const normalizeTime = (value?: number, unit?: 'second' | 'min' | 'hour') => { | |||
| <Form.Item noStyle name={[...names, 'enable']} valuePropName='checked'> | ||
| <Switch size='small' /> | ||
| </Form.Item> |
| <Form.Item noStyle name={[...names, 'resolve_after_enable']} valuePropName='checked'> | ||
| <Checkbox /> | ||
| </Form.Item> |
| <Form.Item noStyle name={[...names, 'resolve_after']} initialValue={1800}> | ||
| <InputNumber min={0} /> | ||
| </Form.Item> |
| <Form.Item | ||
| name={[...names, 'algorithm']} | ||
| rules={enable === true ? [{ required: false, message: t('anomaly_trigger.algorithm_required') }] : []} | ||
| initialValue={_.keys(algorithms)[0]} | ||
| > |
| <Form.Item noStyle name={[...names, 'enable']} valuePropName='checked'> | ||
| <Switch size='small' /> | ||
| </Form.Item> |
…ontainer pattern
Summary by CodeRabbit