feat: optimize workflow page - #2204
Merged
Merged
Conversation
List page: - EmptyGuide empty state with scenario tips + "not active until referenced" hint - processor-chain column, inline enable/disable switch, filter persistence - batch delete + export (drop batch enable/disable: backend lacks the endpoint) - extended case-insensitive search; clone appends "-copy" suffix - drop intentionally-hidden use_case/trigger_mode columns and filters Form page: - restructure into three SectionCards (scope / processors / basic info) - scenario tips card (add only), auto-naming, single-team default select - processor cards: numbered + typed title, collapse summary, drag reorder, delete confirm, type-switch confirm with revert, categorized selector - no-filter warning that escalates when an event_drop processor is present - post-save "go mount" hand-off panel (workflow only runs when a rule references it) - fix P0 data loss: Edit now GET-merge-PUTs so unregistered fields (group_id/use_case/trigger_mode/nodes/connections/inputs) survive edits - fix edit success copy; theme-aware dark-mode CodeMirror background Test modal: - render per-node step timeline (shared NodeResultsSteps) with field-level diffs - surface dropped/failed results + event preview; add fidelity disclaimer Executions page: - EnhancedTable + EmptyGuide; parse trigger_by into alert/notification rule links - add event_id column, error-message preview, event/api mode coloring - detail shows error_node + inputs_snapshot i18n: add keys across zh_CN/en_US/zh_HK/ja_JP/ru_RU. Verified end-to-end on an isolated minimao deploy with per-step DB checks.
…leting enabled workflows
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEvent-pipeline workflows gain sectioned and sortable editing, processor summaries and safeguards, improved list and batch operations, richer execution and test-result views, save guidance, persisted filters, derived-field handling, and expanded localized content. ChangesEvent pipeline experience
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WorkflowForm
participant ProcessorConfig
participant TestModal
participant EventPipelineAPI
participant NodeResultsSteps
WorkflowForm->>ProcessorConfig: configure and reorder processors
ProcessorConfig->>TestModal: provide processor configuration
TestModal->>EventPipelineAPI: submit normalized try-run payload
EventPipelineAPI-->>TestModal: return result data
TestModal->>NodeResultsSteps: render node results
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/pages/eventPipeline/pages/List/index.tsx (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid bypassing the processor contract with
any.Let the callback parameter infer from
Item['processors'](or reuse its exported processor type) so renamed or malformedtyp/typefields are caught statically. As per coding guidelines, “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/eventPipeline/pages/List/index.tsx` at line 39, Update getProcessorTypes to remove the any annotation from the processor callback and use the processor element type inferred from Item['processors'] or its exported processor type. Preserve the existing typ/type extraction and compacting behavior while ensuring invalid processor fields are caught by TypeScript.Source: Coding guidelines
src/pages/eventPipeline/components/NodeResultsSteps.tsx (2)
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
current={_.size(data)}has no effect.Every
Steps.Stepexplicitly setsstatus='finish', which overrides the status antd would otherwise derive fromcurrent. Thecurrentprop is effectively dead here.Also applies to: 47-47
🤖 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/eventPipeline/components/NodeResultsSteps.tsx` at line 43, Remove the ineffective current prop from the Steps component and update each Steps.Step to stop forcing status="finish", allowing antd to derive step status from the active step configuration. Apply the change to both affected step declarations.
31-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
statusMapinto a shared constant. The samerunning/success/failed→ coloredTagmapping is independently re-implemented in three files; one shared helper keeps them in sync and centralizes the unknown-status fallback fix.
src/pages/eventPipeline/components/NodeResultsSteps.tsx#L31-L40: replace the localstatusMap(and ideallyiconMap) with an import from a sharedeventPipelineconstants/util module.src/pages/eventPipeline/pages/Executions/Detail.tsx#L34-L38: replace the localstatusMapwith the same shared export.src/pages/eventPipeline/pages/Executions/index.tsx#L52-L56: replace the localstatusMapwith the same shared export.🤖 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/eventPipeline/components/NodeResultsSteps.tsx` around lines 31 - 40, Extract the duplicated running/success/failed Tag mapping into one shared eventPipeline constants or utility export, including the centralized unknown-status fallback; update statusMap usage in src/pages/eventPipeline/components/NodeResultsSteps.tsx#L31-L40, src/pages/eventPipeline/pages/Executions/Detail.tsx#L34-L38, and src/pages/eventPipeline/pages/Executions/index.tsx#L52-L56 to import that shared export. In NodeResultsSteps.tsx, also reuse a shared iconMap if the module provides one, removing the local mappings while preserving existing translations and styling.src/pages/eventPipeline/pages/Form/index.tsx (1)
69-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd error handling to the team fetch.
getTeamInfoList().then(...)has no rejection handler; a failed request produces an unhandled promise rejection and silently leaves the team select empty. Add a.catchconsistent with the existing pattern.Proposed fix
useEffect(() => { - getTeamInfoList().then((res) => { - const list = res.dat ?? []; - setUserGroups(list); - // 新建（非克隆）且只属于一个团队时，默认选中它，省去新人第一步的卡点 - if (!initialValues && _.isEmpty(form.getFieldValue('team_ids')) && list.length === 1) { - form.setFieldsValue({ team_ids: [list[0].id] }); - } - }); + getTeamInfoList() + .then((res) => { + const list = res.dat ?? []; + setUserGroups(list); + // 新建（非克隆）且只属于一个团队时，默认选中它，省去新人第一步的卡点 + if (!initialValues && _.isEmpty(form.getFieldValue('team_ids')) && list.length === 1) { + form.setFieldsValue({ team_ids: [list[0].id] }); + } + }) + .catch((err) => { + console.error(err); + }); }, []);As per coding guidelines: "Async requests must have error handling (
try/catch,.catch,onError, etc.) consistent with existing patterns".🤖 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/eventPipeline/pages/Form/index.tsx` around lines 69 - 78, Add rejection handling to the getTeamInfoList promise in the Form component’s useEffect, following the existing project pattern for reporting or handling failed requests. Keep the current success flow for setting userGroups and default team_ids unchanged.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.
Inline comments:
In `@src/pages/eventPipeline/components/NodeResultsSteps.tsx`:
- Around line 11-21: Update the status handling in NodeResultsSteps to safely
support unrecognized NodeResult.status values: use an explicit fallback icon and
status label instead of indexing statusMap/iconMap to undefined, and only mark a
Step as finished for recognized success or failure outcomes rather than forcing
status="finish" for every node. Preserve the existing running/success/failed
mappings.
In `@src/pages/eventPipeline/pages/List/index.tsx`:
- Line 209: Replace the href-less anchor in the empty-state help action with the
already imported link-style Button, preserving the existing openDoc click
handler and translated label t('empty_guide.doc').
In `@src/pages/eventPipeline/pages/List/MoreOperations.tsx`:
- Around line 51-59: Update the delete confirmation’s onOk handler in
MoreOperations to re-throw the deleteItems rejection after logging it, so
Modal.confirm keeps the confirmation open on failure. Preserve the existing
success message and onFinished behavior, and optionally display an error toast
before propagating the error.
---
Nitpick comments:
In `@src/pages/eventPipeline/components/NodeResultsSteps.tsx`:
- Line 43: Remove the ineffective current prop from the Steps component and
update each Steps.Step to stop forcing status="finish", allowing antd to derive
step status from the active step configuration. Apply the change to both
affected step declarations.
- Around line 31-40: Extract the duplicated running/success/failed Tag mapping
into one shared eventPipeline constants or utility export, including the
centralized unknown-status fallback; update statusMap usage in
src/pages/eventPipeline/components/NodeResultsSteps.tsx#L31-L40,
src/pages/eventPipeline/pages/Executions/Detail.tsx#L34-L38, and
src/pages/eventPipeline/pages/Executions/index.tsx#L52-L56 to import that shared
export. In NodeResultsSteps.tsx, also reuse a shared iconMap if the module
provides one, removing the local mappings while preserving existing translations
and styling.
In `@src/pages/eventPipeline/pages/Form/index.tsx`:
- Around line 69-78: Add rejection handling to the getTeamInfoList promise in
the Form component’s useEffect, following the existing project pattern for
reporting or handling failed requests. Keep the current success flow for setting
userGroups and default team_ids unchanged.
In `@src/pages/eventPipeline/pages/List/index.tsx`:
- Line 39: Update getProcessorTypes to remove the any annotation from the
processor callback and use the processor element type inferred from
Item['processors'] or its exported processor type. Preserve the existing
typ/type extraction and compacting behavior while ensuring invalid processor
fields are caught by TypeScript.
🪄 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: 74756388-c6ac-4210-9d64-a1cfc4b62788
📒 Files selected for processing (27)
src/components/EmptyGuide/index.tsxsrc/pages/alertRules/FormNG/components/SectionCard/index.tsxsrc/pages/eventPipeline/components/NodeResultsSteps.tsxsrc/pages/eventPipeline/components/SavedGuide.tsxsrc/pages/eventPipeline/components/ScenarioList.tsxsrc/pages/eventPipeline/components/ScenarioTips.tsxsrc/pages/eventPipeline/components/buildWorkflowName.test.tssrc/pages/eventPipeline/components/buildWorkflowName.tssrc/pages/eventPipeline/components/getProcessorSummary.tssrc/pages/eventPipeline/constants.tssrc/pages/eventPipeline/locale/en_US.tssrc/pages/eventPipeline/locale/ja_JP.tssrc/pages/eventPipeline/locale/ru_RU.tssrc/pages/eventPipeline/locale/zh_CN.tssrc/pages/eventPipeline/locale/zh_HK.tssrc/pages/eventPipeline/pages/Add.tsxsrc/pages/eventPipeline/pages/Edit.tsxsrc/pages/eventPipeline/pages/Executions/Detail.tsxsrc/pages/eventPipeline/pages/Executions/index.tsxsrc/pages/eventPipeline/pages/Form/Processor/EventDrop.tsxsrc/pages/eventPipeline/pages/Form/Processor/index.tsxsrc/pages/eventPipeline/pages/Form/TestModal/index.tsxsrc/pages/eventPipeline/pages/Form/index.tsxsrc/pages/eventPipeline/pages/List/MoreOperations.tsxsrc/pages/eventPipeline/pages/List/index.tsxsrc/pages/eventPipeline/utils/normalizeValues.test.tssrc/pages/eventPipeline/utils/normalizeValues.ts
| > | ||
| {t('common:btn.add')} | ||
| </Button> | ||
| <a onClick={openDoc}>{t('empty_guide.doc')}</a> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a keyboard-accessible help action.
An <a> without href is not reliably focusable or activatable from the keyboard. Use the already imported link-style Button instead.
Proposed fix
- <a onClick={openDoc}>{t('empty_guide.doc')}</a>
+ <Button type='link' onClick={openDoc}>
+ {t('empty_guide.doc')}
+ </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.
Suggested change
| <a onClick={openDoc}>{t('empty_guide.doc')}</a> | |
| <Button type='link' onClick={openDoc}> | |
| {t('empty_guide.doc')} | |
| </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/eventPipeline/pages/List/index.tsx` at line 209, Replace the
href-less anchor in the empty-state help action with the already imported
link-style Button, preserving the existing openDoc click handler and translated
label t('empty_guide.doc').
Comment on lines
+51
to
+59
| onOk: () => { | ||
| return deleteItems(_.map(selectedRows, 'id')) | ||
| .then(() => { | ||
| message.success(t('common:success.delete')); | ||
| onFinished?.(); | ||
| }) | ||
| .catch((err) => { | ||
| console.error(err); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Keep the confirmation open on delete failure. Swallowing the rejection makes Modal.confirm treat onOk as resolved, so the dialog closes even when deleteItems fails. Re-throw after logging, and optionally show an error toast.
🤖 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/eventPipeline/pages/List/MoreOperations.tsx` around lines 51 - 59,
Update the delete confirmation’s onOk handler in MoreOperations to re-throw the
deleteItems rejection after logging it, so Modal.confirm keeps the confirmation
open on failure. Preserve the existing success message and onFinished behavior,
and optionally display an error toast before propagating the error.
- toggleDisabled: fetch the latest detail before PUT instead of sending the page-load snapshot. The backend PUT is a full-field overwrite, so replaying a stale snapshot silently reverted concurrent edits to processors and filters. Also guard per-row against double clicks (the flow is now GET+PUT) and refetch the list afterwards, since the detail API does not fill update_by_nickname and patching the row locally would blank that column. - Track row selection by id and derive rows from the latest list data. Storing record references let the "disable before delete" check read a stale disabled value and be bypassed; batch export had the same problem. - Translate processor types before passing them to Tags. The component short-circuits getLabel for string items, so the column rendered raw backend identifiers in every language. - Auto-naming now respects filter_enable, otherwise a workflow that actually processes every event could keep a name claiming a narrower scope. - Stop swallowing non-validation errors in the save handler.
冲突仅 src/pages/alertRules/FormNG/components/SectionCard/index.tsx 一处： main 上独立实现了同名的 summary prop，与本分支语义一致，差异只在注释措辞和 className。采用 main 的版本——它的 `shrink min-w-0` 才能让 truncate 在 flex 容器里真正生效，本分支的 `shrink-0` 会让长摘要撑开布局而不是省略。 EmptyGuide 的 descriptionClassName 两侧内容完全相同，git 自动合并。 eventPipeline 目录 main 未改动，无冲突。
The three SectionCard call sites hardcoded index={0}/{1}/{2}, so the circled
step number rendered by SectionCard was only correct as long as the literals
happened to match the JSX order. Adding, removing or reordering a section
would silently produce duplicate or missing numbers.
Derive the order from the sections definition instead, matching the six
existing call sites in alertRules FormNG, which all use
index={sectionKeys.indexOf(...)}.
… switch/test modal reset
…pand invalid processors
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/eventPipeline/components/buildWorkflowName.test.ts (1)
74-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the undefined case inside the declared API.
truncateNameacceptsstring, but this test bypasses that contract withundefined as unknown as string. Either remove this case or change the source signature toname?: stringand calltruncateName(undefined)directly.🤖 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/eventPipeline/components/buildWorkflowName.test.ts` around lines 74 - 76, Keep the undefined test aligned with truncateName’s declared API: either remove the undefined case, or update truncateName’s signature to accept an optional name and call truncateName(undefined) without type casts, preserving the expected empty-string result.
🤖 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.
Nitpick comments:
In `@src/pages/eventPipeline/components/buildWorkflowName.test.ts`:
- Around line 74-76: Keep the undefined test aligned with truncateName’s
declared API: either remove the undefined case, or update truncateName’s
signature to accept an optional name and call truncateName(undefined) without
type casts, preserving the expected empty-string result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dbccd311-b84d-4eb7-bb79-f70c78249650
📒 Files selected for processing (16)
src/pages/eventPipeline/components/NodeResultsSteps.tsxsrc/pages/eventPipeline/components/buildWorkflowName.test.tssrc/pages/eventPipeline/components/buildWorkflowName.tssrc/pages/eventPipeline/constants.tssrc/pages/eventPipeline/locale/en_US.tssrc/pages/eventPipeline/locale/ja_JP.tssrc/pages/eventPipeline/locale/ru_RU.tssrc/pages/eventPipeline/locale/zh_CN.tssrc/pages/eventPipeline/locale/zh_HK.tssrc/pages/eventPipeline/pages/Executions/Detail.tsxsrc/pages/eventPipeline/pages/Executions/index.tsxsrc/pages/eventPipeline/pages/Form/Processor/index.tsxsrc/pages/eventPipeline/pages/Form/TestModal/index.tsxsrc/pages/eventPipeline/pages/Form/index.tsxsrc/pages/eventPipeline/pages/List/index.tsxsrc/pages/eventPipeline/utils/normalizeValues.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- src/pages/eventPipeline/utils/normalizeValues.ts
- src/pages/eventPipeline/constants.ts
- src/pages/eventPipeline/locale/zh_HK.ts
- src/pages/eventPipeline/components/buildWorkflowName.ts
- src/pages/eventPipeline/components/NodeResultsSteps.tsx
- src/pages/eventPipeline/pages/Form/TestModal/index.tsx
- src/pages/eventPipeline/locale/ja_JP.ts
- src/pages/eventPipeline/locale/zh_CN.ts
- src/pages/eventPipeline/pages/Executions/index.tsx
- src/pages/eventPipeline/pages/Form/Processor/index.tsx
- src/pages/eventPipeline/locale/en_US.ts
- src/pages/eventPipeline/pages/Form/index.tsx
- src/pages/eventPipeline/pages/Executions/Detail.tsx
- src/pages/eventPipeline/pages/List/index.tsx
- src/pages/eventPipeline/locale/ru_RU.ts
…use AffixWrapper for form footer
…flow UX improvements
Fiona2016
added a commit
that referenced
this pull request
Jul 29, 2026
Conflict in eventPipeline/pages/List/index.tsx: #2204 reordered the columns, moving the enabled-status column below tags/update-time/update-by. Took main's ordering and re-applied `defaultSortOrder: 'descend'` to its dateColumn, rather than keeping this branch's copy of the three columns in their old position. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit