feat: sync ai-dev changes - #2159
Conversation
![flashduty[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
💤 Files with no reviewable changes (5)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughThis PR adds a "cannot delete while enabled" guard to delete actions across multiple list/table pages (AI config skills, alert rules, contacts, event pipeline, notification settings/channels/rules, recording rules, warning shield/subscribe). It introduces a shared ChangesDisable delete for enabled records
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)Not applicable — these are repetitive, homogeneous conditional-disable UI changes across pages without new multi-component control flow. Related issues: None linked in the provided information. Related PRs: None linked in the provided information. Suggested labels: i18n, enhancement, ui Suggested reviewers: None specified. Poem A rabbit hops through tables tall, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
![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 (2)
src/pages/recordingRules/PageTable.tsx (1)
251-274: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGuard the batch-delete path too.
This only blocks the per-row action. The bulk delete item still sends
selectRowKeystodeleteRecordingRule(...)without checking whether any selected rule is enabled, so the new protection can be bypassed from the dropdown.Also applies to: 427-428
🤖 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/recordingRules/PageTable.tsx` around lines 251 - 274, The batch-delete handler in PageTable still bypasses the enabled-rule protection by calling deleteRecordingRule with selectRowKeys directly from the dropdown. Update the bulk delete flow in the same action handler (and the related duplicate path mentioned in the review) to first inspect the selected records and block deletion if any are enabled, using the same guard logic as the per-row delete path before calling deleteRecordingRule.src/pages/aiConfig/skills/pages/SkillDetailPanel.tsx (1)
133-148: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMove the tooltip off the disabled menu item
titleon a disabledMenu.Itemwon’t surface here, and wrapping the disabled item itself inTooltipis unreliable in antd 4.21.0. Use the same pattern assrc/components/EnhancedTable/RowActionCell.tsx: keep the menu item enabled, wrap the label inTooltip, and block the action separately.🤖 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/aiConfig/skills/pages/SkillDetailPanel.tsx` around lines 133 - 148, The delete menu in SkillDetailPanel currently puts the tooltip title on a disabled Menu.Item, which won’t render reliably. Update the Menu.Item around the delete action to follow the same pattern used in EnhancedTable/RowActionCell: keep the item itself enabled, wrap the displayed label/icon content in Tooltip, and separately prevent the delete action when item.enabled is true so the tooltip can still appear.
🧹 Nitpick comments (4)
src/pages/contacts/pages/List.tsx (1)
134-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid raw Chinese text as the i18n key.
t("启用状态下不可删除")uses the literal Chinese message as its own translation key, unlike the semantic English keys used elsewhere in this locale file (title,add_title,edit_title,disabled). This pattern is repeated identically across the other modules in this PR (alertRules, notificationRules, recordingRules, etc.), each hardcoding the same Chinese string as a key. Prefer a semantic key (e.g.delete_disabled_tooltip) for consistency and to avoid subtle bugs if the source Chinese phrasing ever needs to change (the key would need to change everywhere it's used).♻️ Suggested key rename
- disabled: reocrd.hide === false, - tooltip: reocrd.hide === false ? t("启用状态下不可删除") : undefined, + disabled: reocrd.hide === false, + tooltip: reocrd.hide === false ? t('delete_disabled_tooltip') : undefined,🤖 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/contacts/pages/List.tsx` around lines 134 - 135, Replace the raw Chinese translation key used in the tooltip with a semantic i18n key, and update the corresponding locale entries accordingly. In List.tsx, change the t("启用状态下不可删除") usage to a stable key such as delete_disabled_tooltip, and apply the same rename pattern anywhere else in this PR where the same literal Chinese string is used as a translation key (for example in alertRules, notificationRules, and recordingRules).src/pages/notificationRules/pages/List.tsx (1)
238-239: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winQuote style inconsistency.
Same as the notificationChannels list: double quotes on the new
t("启用状态下不可删除")call break from the file's single-quote convention (e.g.t('common:btn.delete')above).As per path instructions, "Follow existing repository format and Prettier configuration; keep changes minimal and do not rewrite unrelated modules."
Proposed fix
- disabled: record.enable === true, - tooltip: record.enable === true ? t("启用状态下不可删除") : undefined, + disabled: record.enable === true, + tooltip: record.enable === true ? t('启用状态下不可删除') : undefined,🤖 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/notificationRules/pages/List.tsx` around lines 238 - 239, The new tooltip translation call in List.tsx breaks the file’s existing single-quote convention. Update the `tooltip` expression in the `record.enable === true` branch to use the same quote style as the surrounding `t(...)` calls in the list component, keeping the change minimal and aligned with the existing formatting conventions.Source: Path instructions
src/pages/notificationChannels/pages/List/index.tsx (1)
206-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winQuote style inconsistency.
New lines use double quotes (
t("启用状态下不可删除")) while the rest of the file consistently uses single quotes (e.g.t('common:btn.delete')on line 204). This will likely be flagged by Prettier/lint on next format pass.As per path instructions, "Follow existing repository format and Prettier configuration; keep changes minimal and do not rewrite unrelated modules."
Proposed fix
- disabled: record.enable === true, - tooltip: record.enable === true ? t("启用状态下不可删除") : undefined, + disabled: record.enable === true, + tooltip: record.enable === true ? t('启用状态下不可删除') : undefined,🤖 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/notificationChannels/pages/List/index.tsx` around lines 206 - 207, The new tooltip string in the notification channels List component uses double quotes, which breaks the file’s existing quote style and may fail lint/Prettier checks. Update the `tooltip` expression in `List/index.tsx` to match the surrounding single-quote style used by `t(...)`, keeping the change minimal and limited to the `record.enable === true` conditional.Source: Path instructions
src/pages/notificationChannels/locale/en_US.ts (1)
163-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew key semantically duplicates existing
delete_disable_first.Line 134 already provides
delete_disable_first: 'The current type is not disabled and cannot be deleted'for the same "cannot delete while active" concept. Consider reusing/aligning with the existing key instead of adding a near-duplicate string, to avoid message drift between the two.🤖 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/notificationChannels/locale/en_US.ts` at line 163, The new locale entry is duplicating the existing “cannot delete while enabled” message already covered by delete_disable_first, so align this translation with the existing key instead of introducing a near-duplicate string. Update the en_US mapping in the notification channels locale to reuse or match delete_disable_first, keeping the wording consistent and avoiding message drift across keys.
🤖 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/help/NotificationSettings/locale/en_US.ts`:
- Line 60: The NotificationSettings locale entry added in en_US is missing
matching translations in ja_JP and ru_RU, so users of those locales will see the
untranslated Chinese text. Add the same key under the locale objects used by the
NotificationSettings locale files for ja_JP and ru_RU, and keep the key name
identical to the one already defined in en_US so the existing locale
registration in locale/index.ts continues to resolve it correctly.
---
Outside diff comments:
In `@src/pages/aiConfig/skills/pages/SkillDetailPanel.tsx`:
- Around line 133-148: The delete menu in SkillDetailPanel currently puts the
tooltip title on a disabled Menu.Item, which won’t render reliably. Update the
Menu.Item around the delete action to follow the same pattern used in
EnhancedTable/RowActionCell: keep the item itself enabled, wrap the displayed
label/icon content in Tooltip, and separately prevent the delete action when
item.enabled is true so the tooltip can still appear.
In `@src/pages/recordingRules/PageTable.tsx`:
- Around line 251-274: The batch-delete handler in PageTable still bypasses the
enabled-rule protection by calling deleteRecordingRule with selectRowKeys
directly from the dropdown. Update the bulk delete flow in the same action
handler (and the related duplicate path mentioned in the review) to first
inspect the selected records and block deletion if any are enabled, using the
same guard logic as the per-row delete path before calling deleteRecordingRule.
---
Nitpick comments:
In `@src/pages/contacts/pages/List.tsx`:
- Around line 134-135: Replace the raw Chinese translation key used in the
tooltip with a semantic i18n key, and update the corresponding locale entries
accordingly. In List.tsx, change the t("启用状态下不可删除") usage to a stable key such
as delete_disabled_tooltip, and apply the same rename pattern anywhere else in
this PR where the same literal Chinese string is used as a translation key (for
example in alertRules, notificationRules, and recordingRules).
In `@src/pages/notificationChannels/locale/en_US.ts`:
- Line 163: The new locale entry is duplicating the existing “cannot delete
while enabled” message already covered by delete_disable_first, so align this
translation with the existing key instead of introducing a near-duplicate
string. Update the en_US mapping in the notification channels locale to reuse or
match delete_disable_first, keeping the wording consistent and avoiding message
drift across keys.
In `@src/pages/notificationChannels/pages/List/index.tsx`:
- Around line 206-207: The new tooltip string in the notification channels List
component uses double quotes, which breaks the file’s existing quote style and
may fail lint/Prettier checks. Update the `tooltip` expression in
`List/index.tsx` to match the surrounding single-quote style used by `t(...)`,
keeping the change minimal and limited to the `record.enable === true`
conditional.
In `@src/pages/notificationRules/pages/List.tsx`:
- Around line 238-239: The new tooltip translation call in List.tsx breaks the
file’s existing single-quote convention. Update the `tooltip` expression in the
`record.enable === true` branch to use the same quote style as the surrounding
`t(...)` calls in the list component, keeping the change minimal and aligned
with the existing formatting conventions.
🪄 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: d382f873-63c0-4f39-8a4b-40278e78dcc6
📒 Files selected for processing (41)
src/pages/aiConfig/skills/locale/en_US.tssrc/pages/aiConfig/skills/locale/zh_CN.tssrc/pages/aiConfig/skills/locale/zh_HK.tssrc/pages/aiConfig/skills/pages/SkillDetailPanel.tsxsrc/pages/alertRules/List/ListNG.tsxsrc/pages/alertRules/locale/en_US.tssrc/pages/alertRules/locale/zh_CN.tssrc/pages/alertRules/locale/zh_HK.tssrc/pages/contacts/locale/en_US.tssrc/pages/contacts/locale/zh_CN.tssrc/pages/contacts/locale/zh_HK.tssrc/pages/contacts/pages/List.tsxsrc/pages/eventPipeline/locale/en_US.tssrc/pages/eventPipeline/locale/zh_CN.tssrc/pages/eventPipeline/locale/zh_HK.tssrc/pages/eventPipeline/pages/List/index.tsxsrc/pages/help/NotificationSettings/Channels/index.tsxsrc/pages/help/NotificationSettings/Contacts/index.tsxsrc/pages/help/NotificationSettings/locale/en_US.tssrc/pages/help/NotificationSettings/locale/zh_CN.tssrc/pages/help/NotificationSettings/locale/zh_HK.tssrc/pages/notificationChannels/locale/en_US.tssrc/pages/notificationChannels/locale/zh_CN.tssrc/pages/notificationChannels/locale/zh_HK.tssrc/pages/notificationChannels/pages/List/index.tsxsrc/pages/notificationRules/locale/en_US.tssrc/pages/notificationRules/locale/zh_CN.tssrc/pages/notificationRules/locale/zh_HK.tssrc/pages/notificationRules/pages/List.tsxsrc/pages/recordingRules/PageTable.tsxsrc/pages/recordingRules/locale/en_US.tssrc/pages/recordingRules/locale/zh_CN.tssrc/pages/recordingRules/locale/zh_HK.tssrc/pages/warning/shield/index.tsxsrc/pages/warning/shield/locale/en_US.tssrc/pages/warning/shield/locale/zh_CN.tssrc/pages/warning/shield/locale/zh_HK.tssrc/pages/warning/subscribe/ListNG.tsxsrc/pages/warning/subscribe/locale/en_US.tssrc/pages/warning/subscribe/locale/zh_CN.tssrc/pages/warning/subscribe/locale/zh_HK.ts
| ibex: { | ||
| title: 'Ibex settings', | ||
| }, | ||
| 启用状态下不可删除: 'Cannot delete while enabled. Disable first.', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'src/pages/help/NotificationSettings/locale/*' 'src/pages/help/NotificationSettings/**'
printf '\n== locale index ==\n'
sed -n '1,200p' src/pages/help/NotificationSettings/locale/index.ts
printf '\n== en_US ==\n'
sed -n '1,140p' src/pages/help/NotificationSettings/locale/en_US.ts
printf '\n== ja_JP ==\n'
sed -n '1,140p' src/pages/help/NotificationSettings/locale/ja_JP.ts
printf '\n== ru_RU ==\n'
sed -n '1,140p' src/pages/help/NotificationSettings/locale/ru_RU.ts
printf '\n== search for key across namespace ==\n'
rg -n "启用状态下不可删除|Cannot delete while enabled|notificationSettings" src/pages/help/NotificationSettings -SRepository: n9e/fe
Length of output: 11786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file sizes ==\n'
wc -l src/pages/help/NotificationSettings/locale/en_US.ts src/pages/help/NotificationSettings/locale/ja_JP.ts src/pages/help/NotificationSettings/locale/ru_RU.ts src/pages/help/NotificationSettings/locale/index.ts
printf '\n== recent diff for locale files ==\n'
git diff -- src/pages/help/NotificationSettings/locale/en_US.ts src/pages/help/NotificationSettings/locale/ja_JP.ts src/pages/help/NotificationSettings/locale/ru_RU.ts src/pages/help/NotificationSettings/locale/index.tsRepository: n9e/fe
Length of output: 442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== i18n config files ==\n'
git ls-files | rg '(^|/)i18n|(^|/)locales|translation|react-i18next|i18next' -n
printf '\n== search for fallback and interpolation config ==\n'
rg -n "fallbackLng|fallbackNS|missingKey|returnNull|returnEmptyString|saveMissing|interpolation|defaultNS|ns:" src . -g '!**/node_modules/**' -S
printf '\n== search for useTranslation setup ==\n'
rg -n "useTranslation\\(" src -SRepository: n9e/fe
Length of output: 50363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== notification settings zh locales ==\n'
sed -n '1,140p' src/pages/help/NotificationSettings/locale/zh_CN.ts
printf '\n--- zh_HK ---\n'
sed -n '1,140p' src/pages/help/NotificationSettings/locale/zh_HK.ts
printf '\n== i18n init candidates ==\n'
fd -a -H -I 'i18n*.ts' src . || true
fd -a -H -I 'i18n*.tsx' src . || true
fd -a -H -I 'i18n*.js' src . || true
fd -a -H -I 'i18n*.jsx' src . || true
printf '\n== direct fallback search in likely files ==\n'
rg -n "fallbackLng|defaultNS|fallbackNS|returnNull|returnEmptyString|saveMissing|interpolation" src/{locales,components,pages,plugins} -SRepository: n9e/fe
Length of output: 5935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' src/i18n.tsRepository: n9e/fe
Length of output: 3626
Mirror this key in ja_JP and ru_RU. src/pages/help/NotificationSettings/locale/index.ts registers both locales, and src/i18n.ts loads each language separately without a fallback config, so missing entries will show the raw Chinese key for those users.
🤖 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/help/NotificationSettings/locale/en_US.ts` at line 60, The
NotificationSettings locale entry added in en_US is missing matching
translations in ja_JP and ru_RU, so users of those locales will see the
untranslated Chinese text. Add the same key under the locale objects used by the
NotificationSettings locale files for ja_JP and ru_RU, and keep the key name
identical to the one already defined in en_US so the existing locale
registration in locale/index.ts continues to resolve it correctly.
2 participants
Summary
Back to home/回到首页/回到首頁wording per review.prehas been merged and pushed; the zh locale conflict was resolved by keeping both theform_ngblock and delete guard copy.Verification
npm run buildpassed on the feature branch after review fixes.git diff --checkpassed.Summary by CodeRabbit