feat: add quick-create notification rule from pasted webhook/integration URL - #2217
Conversation
📝 WalkthroughWalkthroughAdds webhook-based quick creation for notification rules, including provider parsing, channel/rule reuse or provisioning, a modal workflow, selector integration, automatic selection, validation tests, and localized UI text. ChangesNotification rule quick creation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RuleDropdownSelect
participant QuickCreateModal
participant quickCreateNotifyRule
participant NotificationServices
RuleDropdownSelect->>QuickCreateModal: open quick-create
QuickCreateModal->>quickCreateNotifyRule: submit webhook and rule fields
quickCreateNotifyRule->>NotificationServices: reuse or create channel and rule
NotificationServices-->>quickCreateNotifyRule: return creation result
quickCreateNotifyRule-->>QuickCreateModal: return ruleId and reused
QuickCreateModal-->>RuleDropdownSelect: refresh and select rule
Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/pages/notificationRules/components/RuleDropdownSelect/quickCreate.ts (1)
46-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate "last-4-of-token" suffix logic across two files.
quickCreate.tsdefines this exact truncation rule in a privatetokenSuffixhelper, andQuickCreateModal.tsxreimplements it inline for the "detected" hint instead of importing it.
src/pages/notificationRules/components/RuleDropdownSelect/quickCreate.ts#L46-L48: exporttokenSuffix(export function tokenSuffix(token: string) { ... }).src/pages/notificationRules/components/RuleDropdownSelect/QuickCreateModal.tsx#L94-L106: importtokenSuffixfrom./quickCreateand replacetoken.length >= 4 ? token.slice(-4) : tokenwithtokenSuffix(token).🤖 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/components/RuleDropdownSelect/quickCreate.ts` around lines 46 - 48, Export the tokenSuffix helper in quickCreate.ts so it can be reused. In QuickCreateModal.tsx, import tokenSuffix from ./quickCreate and replace the inline last-four-character truncation in the detected hint with tokenSuffix(token), preserving the existing behavior at both sites.
🤖 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/notificationRules/components/RuleDropdownSelect/index.tsx`:
- Around line 326-331: Replace the toolbar quick-create `<a>` in the authorized
branch with the same `<Button type="link">` pattern used by the empty-state
action, preserving `handleQuickCreate`, the icon, and translated label so the
trigger is keyboard accessible.
- Around line 388-397: Replace the inline ID extraction in the
RuleDropdownSelect success handler with the exported extractId helper from
quickCreate.ts. Reuse that shared helper for the postItems response, then retain
the existing finite-positive check and selectRule call.
In `@src/pages/notificationRules/components/RuleDropdownSelect/quickCreate.ts`:
- Around line 201-203: Replace the substring-based Flashduty integration lookup
with an exact comparison of the stored integration key against parsed.token in
findRuleByToken and the related quick-create flow. Reuse
flashdutyIntegrationUrlOf only if it returns the exact key, preserving the
existing IM-provider matching behavior and preventing matches caused by URL
suffixes or query parameters.
In
`@src/pages/notificationRules/components/RuleDropdownSelect/QuickCreateModal.tsx`:
- Around line 36-40: Reuse the memoized parseResult from the component’s useMemo
in both suggestQuickRuleName/handleUrlChange and the webhook URL validator
instead of calling tryParseWebhookInput repeatedly; ensure name suggestion and
validation use the current parsed value, accounting for watch-driven update
timing.
- Around line 42-54: Update the visible-state effect in QuickCreateModal to
clear the teams state when the modal is closed, while preserving the existing
form reset and team-loading behavior when visible is true. Ensure the close path
explicitly resets this temporary local state before returning.
---
Nitpick comments:
In `@src/pages/notificationRules/components/RuleDropdownSelect/quickCreate.ts`:
- Around line 46-48: Export the tokenSuffix helper in quickCreate.ts so it can
be reused. In QuickCreateModal.tsx, import tokenSuffix from ./quickCreate and
replace the inline last-four-character truncation in the detected hint with
tokenSuffix(token), preserving the existing behavior at both sites.
🪄 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 Plus
Run ID: bfdde92c-d11e-4dba-9af5-0ff25473755f
📒 Files selected for processing (9)
src/pages/notificationRules/components/RuleDropdownSelect/QuickCreateModal.tsxsrc/pages/notificationRules/components/RuleDropdownSelect/index.tsxsrc/pages/notificationRules/components/RuleDropdownSelect/quickCreate.test.tssrc/pages/notificationRules/components/RuleDropdownSelect/quickCreate.tssrc/pages/notificationRules/locale/en_US.tssrc/pages/notificationRules/locale/ja_JP.tssrc/pages/notificationRules/locale/ru_RU.tssrc/pages/notificationRules/locale/zh_CN.tssrc/pages/notificationRules/locale/zh_HK.ts
| {isAuthorized && ( | ||
| <a onClick={handleQuickCreate}> | ||
| <ThunderboltOutlined className='mr-1' /> | ||
| {t('rule_select.quick_create.action')} | ||
| </a> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Quick-create toolbar trigger uses <a> without href, breaking keyboard access.
The identical action is exposed as <Button type='link'> in the empty-state (lines 277-279), which is keyboard-focusable/activatable. This <a onClick={...}> with no href isn't part of the native tab order and can't be triggered via keyboard (Enter) in most browsers, unlike a real link or a <Button>.
🛠️ Proposed fix: use the same `Button type='link'` pattern used elsewhere in this file
- {isAuthorized && (
- <a onClick={handleQuickCreate}>
- <ThunderboltOutlined className='mr-1' />
- {t('rule_select.quick_create.action')}
- </a>
- )}
+ {isAuthorized && (
+ <Button type='link' size='small' icon={<ThunderboltOutlined />} onClick={handleQuickCreate}>
+ {t('rule_select.quick_create.action')}
+ </Button>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {isAuthorized && ( | |
| <a onClick={handleQuickCreate}> | |
| <ThunderboltOutlined className='mr-1' /> | |
| {t('rule_select.quick_create.action')} | |
| </a> | |
| )} | |
| {isAuthorized && ( | |
| <Button type='link' size='small' icon={<ThunderboltOutlined />} onClick={handleQuickCreate}> | |
| {t('rule_select.quick_create.action')} | |
| </Button> | |
| )} |
🤖 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/components/RuleDropdownSelect/index.tsx` around
lines 326 - 331, Replace the toolbar quick-create `<a>` in the authorized branch
with the same `<Button type="link">` pattern used by the empty-state action,
preserving `handleQuickCreate`, the icon, and translated label so the trigger is
keyboard accessible.
| .then((dat) => { | ||
| message.success(t('common:success.add')); | ||
| handleCloseCreateDrawer(); | ||
| refresh?.(); | ||
| // 自动选中新建的规则,免去用户再去下拉里手动勾选 | ||
| const first = Array.isArray(dat) ? dat[0] : undefined; | ||
| const newId = typeof first === 'object' ? Number(first?.id) : Number(first); | ||
| if (Number.isFinite(newId) && newId > 0) { | ||
| selectRule(newId); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Duplicated, divergent id-extraction logic vs. quickCreate.ts's extractId.
This inline extraction sets first = undefined when dat isn't an array, while quickCreate.ts's extractId (used against the very same postItems API) falls back to first = dat. Both consume the same create-rule API response shape, so any divergence in actual response format silently disables auto-select here but not in quickCreate.ts. Export and reuse extractId instead of re-implementing it.
♻️ Proposed fix
// quickCreate.ts
-function extractId(dat: any): number | undefined {
+export function extractId(dat: any): number | undefined { // index.tsx
+import { extractId } from './quickCreate';
...
createNotificationRules([values])
.then((dat) => {
message.success(t('common:success.add'));
handleCloseCreateDrawer();
refresh?.();
// 自动选中新建的规则,免去用户再去下拉里手动勾选
- const first = Array.isArray(dat) ? dat[0] : undefined;
- const newId = typeof first === 'object' ? Number(first?.id) : Number(first);
- if (Number.isFinite(newId) && newId > 0) {
- selectRule(newId);
- }
+ const newId = extractId(dat);
+ if (newId) {
+ selectRule(newId);
+ }
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .then((dat) => { | |
| message.success(t('common:success.add')); | |
| handleCloseCreateDrawer(); | |
| refresh?.(); | |
| // 自动选中新建的规则,免去用户再去下拉里手动勾选 | |
| const first = Array.isArray(dat) ? dat[0] : undefined; | |
| const newId = typeof first === 'object' ? Number(first?.id) : Number(first); | |
| if (Number.isFinite(newId) && newId > 0) { | |
| selectRule(newId); | |
| } | |
| .then((dat) => { | |
| message.success(t('common:success.add')); | |
| handleCloseCreateDrawer(); | |
| refresh?.(); | |
| // 自动选中新建的规则,免去用户再去下拉里手动勾选 | |
| const newId = extractId(dat); | |
| if (newId) { | |
| selectRule(newId); | |
| } |
🤖 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/components/RuleDropdownSelect/index.tsx` around
lines 388 - 397, Replace the inline ID extraction in the RuleDropdownSelect
success handler with the exported extractId helper from quickCreate.ts. Reuse
that shared helper for the postItems response, then retain the existing
finite-positive check and selectRule call.
| function flashdutyIntegrationUrlOf(channel: any): string { | ||
| return String(channel?.request_config?.flashduty_request_config?.integration_url || '').trim(); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Flashduty dedup uses substring match on integration_key, risking false-positive reuse.
flashdutyIntegrationUrlOf(c).includes(parsed.token) is a substring check, unlike the exact match used for IM providers in findRuleByToken (String(c) === token). If one integration key happens to be a substring of another channel's stored integration_url (or the URL contains extra query params), this will incorrectly report reused: true against the wrong rule, silently skipping creation of the user's actual intended notification path.
🐛 Proposed fix: exact-match the integration_key instead of substring search
function flashdutyIntegrationUrlOf(channel: any): string {
return String(channel?.request_config?.flashduty_request_config?.integration_url || '').trim();
}
+
+function flashdutyIntegrationKeyOf(channel: any): string {
+ const url = flashdutyIntegrationUrlOf(channel);
+ try {
+ return new URL(url).searchParams.get('integration_key') || '';
+ } catch {
+ return '';
+ }
+}- if (isFlashduty && flashdutyIntegrationUrlOf(c).includes(parsed.token) && c?.id != null) {
+ if (isFlashduty && flashdutyIntegrationKeyOf(c) === parsed.token && c?.id != null) {Also applies to: 284-306
🤖 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/components/RuleDropdownSelect/quickCreate.ts`
around lines 201 - 203, Replace the substring-based Flashduty integration lookup
with an exact comparison of the stored integration key against parsed.token in
findRuleByToken and the related quick-create flow. Reuse
flashdutyIntegrationUrlOf only if it returns the exact key, preserving the
existing IM-provider matching behavior and preventing matches caused by URL
suffixes or query parameters.
| const parseResult = useMemo(() => { | ||
| const trimmed = _.trim(urlValue || ''); | ||
| if (!trimmed) return undefined; | ||
| return tryParseWebhookInput(trimmed); | ||
| }, [urlValue]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Webhook URL is re-parsed independently in three places per keystroke.
parseResult (36-40) already recomputes tryParseWebhookInput reactively via Form.useWatch, yet handleUrlChange (via suggestQuickRuleName, line 57) and the field validator (135-137) each call tryParseWebhookInput again on the same value. Reuse the memoized parseResult instead of recomputing.
♻️ Proposed fix: reuse `parseResult` in the validator and name suggestion
const handleUrlChange = (value: string) => {
- const suggestion = suggestQuickRuleName(value);
- if (!suggestion) return;
+ const trimmed = _.trim(value || '');
+ const result = tryParseWebhookInput(trimmed);
+ if (!result.ok) return;
+ const suggestion = `${result.parsed.channelName}-${tokenSuffix(result.parsed.token)}`;
const current = form.getFieldValue('name');
if (current && current !== lastAutoNameRef.current) return;
form.setFieldsValue({ name: suggestion });
lastAutoNameRef.current = suggestion;
}; {
validator: (_rule, value) => {
const trimmed = _.trim(value || '');
if (!trimmed) return Promise.resolve();
- const result = tryParseWebhookInput(trimmed);
- return result.ok ? Promise.resolve() : Promise.reject(new Error(result.error));
+ return parseResult?.ok || !trimmed ? Promise.resolve() : Promise.reject(new Error((parseResult as { ok: false; error: string } | undefined)?.error));
},
},(Note: validator timing vs. the watch-driven parseResult update should be double-checked; a simpler alternative is to keep the validator's own call but drop the separate suggestQuickRuleName call in handleUrlChange in favor of deriving the name from parseResult.)
Also applies to: 56-63, 125-139
🤖 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/components/RuleDropdownSelect/QuickCreateModal.tsx`
around lines 36 - 40, Reuse the memoized parseResult from the component’s
useMemo in both suggestQuickRuleName/handleUrlChange and the webhook URL
validator instead of calling tryParseWebhookInput repeatedly; ensure name
suggestion and validation use the current parsed value, accounting for
watch-driven update timing.
Source: Coding guidelines
Summary by CodeRabbit