feat: revamp alert mute form with sections, auto-naming, quick durations and empty guide - #2197
Conversation
…ons and empty guide
📝 WalkthroughWalkthroughThe Shield rule form is refactored into collapsible sections with generated summaries, quick-duration and periodic mute controls, centralized validation, preview feedback, and expanded localization. The Shield list gains improved loading, documentation, add eligibility, and empty-state behavior. ChangesShield rule configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OperateForm
participant SectionCard
participant PreviewMutedEvents
User->>OperateForm: Submit preview or test
OperateForm->>OperateForm: Validate fields
OperateForm->>SectionCard: Expand sections containing errors
OperateForm->>PreviewMutedEvents: Continue preview flow
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/warning/shield/components/PreviewMutedEvents.tsx (1)
122-124: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear local state when the modal closes.
As per coding guidelines, modal components must explicitly clean up local state (such as temporary selection values) on unmount to prevent stale data from appearing on re-open. If a user selects rows, cancels, and then triggers the preview again, the old
selectedRowKeyswill persist, which could lead to accidental deletions of unrelated events.Use the
afterCloseprop on theModalto reliably clear the local states.🧹 Proposed fix
<Modal title={t('alertMutes:preview_muted_title')} visible={visible} + afterClose={() => { + setSelectedRowKeys([]); + setData([]); + }} footer={[🤖 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/warning/shield/components/PreviewMutedEvents.tsx` around lines 122 - 124, Update the Modal in PreviewMutedEvents to use its afterClose callback to reset selectedRowKeys and any other temporary local state, ensuring cancelled or reopened previews never retain stale selections.Source: Coding guidelines
🧹 Nitpick comments (1)
src/pages/warning/shield/components/utils.ts (1)
21-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the interface to reflect runtime types.
The
formatMuteTagfunction below checks iftag.valueis an array, but the interface explicitly types it asstring. To prevent TypeScript errors for consumers passing array values (e.g., forinornot inoperators), consider updating the type.💡 Proposed fix
export interface MuteTagItem { key?: string; func?: string; - value?: string; + value?: string | string[]; }🤖 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/warning/shield/components/utils.ts` around lines 21 - 25, Update the MuteTagItem.value property type to include the array shape accepted by formatMuteTag, while preserving support for string and optional values. Ensure the interface matches runtime inputs used by in and not in operators so consumers can pass arrays without TypeScript errors.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/pages/warning/shield/locale/en_US.ts`:
- Around line 27-48: Add the missing mute_type.days_of_week_msg translation to
the mute_type locale block in src/pages/warning/shield/locale/en_US.ts (lines
27-48), src/pages/warning/shield/locale/ja_JP.ts (lines 27-48), and
src/pages/warning/shield/locale/ru_RU.ts (lines 27-43), providing the required
periodic days-of-week validation message in each locale.
---
Outside diff comments:
In `@src/pages/warning/shield/components/PreviewMutedEvents.tsx`:
- Around line 122-124: Update the Modal in PreviewMutedEvents to use its
afterClose callback to reset selectedRowKeys and any other temporary local
state, ensuring cancelled or reopened previews never retain stale selections.
---
Nitpick comments:
In `@src/pages/warning/shield/components/utils.ts`:
- Around line 21-25: Update the MuteTagItem.value property type to include the
array shape accepted by formatMuteTag, while preserving support for string and
optional values. Ensure the interface matches runtime inputs used by in and not
in operators so consumers can pass arrays without TypeScript errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 662c066a-abe2-4481-b0c9-6e4d7572ff48
📒 Files selected for processing (13)
src/pages/alertRules/FormNG/components/SectionCard/index.tsxsrc/pages/warning/shield/add.tsxsrc/pages/warning/shield/components/DaysOfWeekSelect.tsxsrc/pages/warning/shield/components/PreviewMutedEvents.tsxsrc/pages/warning/shield/components/operateForm.tsxsrc/pages/warning/shield/components/utils.summary.test.tssrc/pages/warning/shield/components/utils.tssrc/pages/warning/shield/index.tsxsrc/pages/warning/shield/locale/en_US.tssrc/pages/warning/shield/locale/ja_JP.tssrc/pages/warning/shield/locale/ru_RU.tssrc/pages/warning/shield/locale/zh_CN.tssrc/pages/warning/shield/locale/zh_HK.ts
| mute_type: { | ||
| label: 'Mute type', | ||
| 0: 'Fixed time', | ||
| 1: 'Periodic time', | ||
| days_of_week: 'Mute time', | ||
| days_preset: { | ||
| everyday: 'Every day', | ||
| workday: 'Workdays', | ||
| weekend: 'Weekend', | ||
| }, | ||
| start: 'Start time', | ||
| start_msg: 'Start time is required', | ||
| end: 'End time', | ||
| end_msg: 'End time is required', | ||
| periodic_tip: 'A periodic mute never expires, alerts matched in the time ranges above are muted every week. Same start and end time means the whole day', | ||
| }, | ||
| mute_method: { | ||
| label: 'Mute method', | ||
| 0: 'Mute events and notifications', | ||
| '0_desc': '(no event, no notification)', | ||
| 1: 'Mute notifications only', | ||
| '1_desc': '(events are still recorded)', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify days_of_week_msg exists across all shield locale files
fd . src/pages/warning/shield/locale --extension ts \
--exec sh -c 'echo "== $1 =="; rg -n "days_of_week_msg|days_of_week\b" "$1"' _ {}Repository: n9e/fe
Length of output: 592
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== operateForm usage =="
rg -n "days_of_week_msg|mute_type\.days_of_week|periodic" src/pages/warning/shield -S
echo
echo "== locale files with mute_type blocks =="
for f in src/pages/warning/shield/locale/*.ts; do
echo "---- $f ----"
sed -n '20,60p' "$f"
doneRepository: n9e/fe
Length of output: 12248
Add mute_type.days_of_week_msg to the shield locales. operateForm.tsx uses this key for the periodic days-of-week required message, and all shield locale blocks (en_US.ts, ja_JP.ts, ru_RU.ts, zh_CN.ts, zh_HK.ts) still omit it, so an empty value falls back to the raw key string.
📍 Affects 3 files
src/pages/warning/shield/locale/en_US.ts#L27-L48(this comment)src/pages/warning/shield/locale/ja_JP.ts#L27-L48src/pages/warning/shield/locale/ru_RU.ts#L27-L43
🤖 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/warning/shield/locale/en_US.ts` around lines 27 - 48, Add the
missing mute_type.days_of_week_msg translation to the mute_type locale block in
src/pages/warning/shield/locale/en_US.ts (lines 27-48),
src/pages/warning/shield/locale/ja_JP.ts (lines 27-48), and
src/pages/warning/shield/locale/ru_RU.ts (lines 27-43), providing the required
periodic days-of-week validation message in each locale.
- 固定时间模式下周期屏蔽区块被隐藏，其 required 校验也随之关闭， 避免用户切到周期模式加了空行再切回固定时间后，保存被隐藏字段拦住且看不到错误 - 「全部业务组」或非叶子节点下的空态提示改为指向具体业务组，五种语言同步
- 事件等级未全选也算一种筛选，与订阅规则口径一致， 避免只收敛级别时仍提示「将屏蔽所有告警事件」 - 起止时间联动的 validateFields(['etime']) 与预览保存回调补上 catch， 避免校验失败时控制台出现 Uncaught (in promise) 报错
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/warning/shield/components/operateForm.tsx (1)
152-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMatch day presets with calendar arithmetic
activeQuickDurationcompares elapsed milliseconds, butquickDurationChangebuilds1d/3d/7d/30dwithmoment.add, which preserves wall-clock time across DST. That can leave a day-based preset unselected around DST boundaries. Compare againstmoment(btime).clone().add(num, unit)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/warning/shield/components/operateForm.tsx` around lines 152 - 158, Update the activeQuickDuration matcher to use calendar arithmetic consistent with quickDurationChange: clone moment(btime), add the parsed num and unit, and compare the resulting time with etime instead of comparing elapsed milliseconds. Preserve the existing undefined behavior when btime or etime is missing and the QUICK_DURATIONS lookup.
🧹 Nitpick comments (2)
src/pages/warning/shield/components/operateForm.tsx (2)
384-384: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Tailwind utilities for the new layout rules.
The new visibility, widths, flex layout, and spacing are directly expressible with Tailwind; keeping them inline fragments the component styling approach.
src/pages/warning/shield/components/operateForm.tsx#L384-L384: replace the inline display toggle with conditionalhidden/blockclasses.src/pages/warning/shield/components/operateForm.tsx#L435-L435: replace the inline display toggle with conditionalhidden/blockclasses.src/pages/warning/shield/components/operateForm.tsx#L440-L469: replace width, flex, and bottom-margin styles with Tailwind utilities.src/pages/warning/shield/components/operateForm.tsx#L475-L501: replace fixed-widthForm.Itemstyles with Tailwind width utilities.As per coding guidelines, use Tailwind utility classes for container layout and component-specific display styles.
🤖 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/warning/shield/components/operateForm.tsx` at line 384, Update operateForm.tsx at lines 384 and 435 to replace inline display toggles with conditional hidden/block Tailwind classes; replace inline width, flex, and margin styles at lines 440-469 with equivalent Tailwind utilities; and replace fixed-width Form.Item styles at lines 475-501 with Tailwind width utilities.Source: Coding guidelines
63-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the declared props type.
Propsis defined but bypassed with: any, sodetailandtypelose type checking inside the component. Type the destructured parameter asPropsand make the defaultdetailvalue conform to its declared shape.As per coding guidelines, Component Props must use
interfacefor explicit declaration and 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/warning/shield/components/operateForm.tsx` at line 63, Update the OperateForm component to destructure its parameter as the declared Props type instead of any, and ensure the default detail value matches Props.detail’s declared shape. Define Props as an interface if it is currently another declaration form, preserving type checking for both detail and type.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/pages/warning/shield/components/operateForm.tsx`:
- Around line 152-158: Update the activeQuickDuration matcher to use calendar
arithmetic consistent with quickDurationChange: clone moment(btime), add the
parsed num and unit, and compare the resulting time with etime instead of
comparing elapsed milliseconds. Preserve the existing undefined behavior when
btime or etime is missing and the QUICK_DURATIONS lookup.
---
Nitpick comments:
In `@src/pages/warning/shield/components/operateForm.tsx`:
- Line 384: Update operateForm.tsx at lines 384 and 435 to replace inline
display toggles with conditional hidden/block Tailwind classes; replace inline
width, flex, and margin styles at lines 440-469 with equivalent Tailwind
utilities; and replace fixed-width Form.Item styles at lines 475-501 with
Tailwind width utilities.
- Line 63: Update the OperateForm component to destructure its parameter as the
declared Props type instead of any, and ensure the default detail value matches
Props.detail’s declared shape. Define Props as an interface if it is currently
another declaration form, preserving type checking for both detail and type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bc349be-e926-499a-9d9f-f15d11694ae7
📒 Files selected for processing (3)
src/pages/warning/shield/components/operateForm.tsxsrc/pages/warning/shield/components/utils.summary.test.tssrc/pages/warning/shield/components/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/warning/shield/components/utils.summary.test.ts
- src/pages/warning/shield/components/utils.ts
2 participants
Summary by CodeRabbit