feat(table): align table design in fe#2102
Open
Fiona2016 wants to merge 46 commits into
Open
Conversation
Per design-system-table.md token baseline, sorted body cells use rgb(var(--fc-fill-5-rgb) / 0.1), not the header bg (fill-2-5).
Short headers (≤4 chars) must not wrap; wide tables use horizontal scroll instead of squeezing the header.
…ob-tpls Per design-system-table.md §8: default pageSize is 15 across all tables. - alert-mutes (shield): drop defaultPageSize 30 override; inherit DEFAULT_PAGESIZE. - alert-subscribes (ListNG): drop defaultPageSize 30 override; inherit DEFAULT_PAGESIZE. - job-tpls: add 15 to pageSizeOptions so it is selectable.
Per design-system-table.md §17.3 PRD scope: - Column1Title: 订阅名称 - Column1Content: 业务组 Drop the standalone business_group column; show group as a sub-line under the subscribe name using the double-row identifier pattern (design-system §5).
…to Column1 Per design-system-table.md §17.3: - Column1Title: 规则标题 - Column1Content: 数据源类型 + 业务组 Drop the standalone business_group and cate columns; surface them as a sub-line beneath the rule title.
Per design-system-table.md §17.3: - Column1Title: 标题 - Column1Content: ID + 业务组
Per design-system-table.md §17.3: - Column1Title: 标题 - Column1Content: ID + 业务组
Per design-system-table.md §17.3: - Column1Title: 用户名 - Column1Content: 显示名 + 邮箱 + 手机号 Username keeps sorter; secondary identifiers shown beneath as text-soft.
…to Column2 Per design-system-table.md §17.3: - Column1: 状态 (kept as-is) - Column2Title: 名称 - Column2Content: 类型 (logo) + 业务组 + 级别 - Column3: 数据源 Drop standalone cate, group_id, severities columns; embed them under name. Trim defaultColumnsConfigs accordingly.
…-0509 # Conflicts: # src/pages/task/index.tsx # src/pages/taskTpl/index.tsx # src/pages/warning/shield/index.tsx
Sorted column TD was using antd default #fafafa while sorted TH inherited a different bg, making the column look split. Apply the same fc-fill-5 token to both so the whole column reads as one band. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Remove `visibility: collapse` on the rc-table measure row — Chrome treats it as collapsing layout, so MeasureCell reports offsetWidth=0 when there is no data. Combined with the recent fixed-right + scroll x:max-content rollout, empty-state tables had the first column absorb all available width. Inline `height:0; fontSize:0` on the tr already hides the row, so the rule was redundant anyway. Also flip the ping-right shadow gradient so the darker edge sits next to the fixed column instead of away from it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughAdds shared TableActionDropdown and TableTags components, migrates many table UIs to dropdown-driven actions and constrained tag rendering, removes legacy per-page tag components, updates sorter visuals and theme variables, and sets Table default sortDirections. ChangesComprehensive Table UI Refactoring
Estimated code review effort: 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major 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 (3)
src/pages/warning/subscribe/ListNG.tsx (1)
311-318:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix delete failure handling and toast severity.
When
res.errexists, it currently shows a success toast, and rejected requests are not caught.Suggested fix
- deleteSubscribes({ ids: [record.id] }, record.group_id).then((res) => { - refreshList(); - if (res.err) { - message.success(res.err); - } else { - message.success(t('common:success.delete')); - } - }); + return deleteSubscribes({ ids: [record.id] }, record.group_id) + .then((res) => { + refreshList(); + if (res.err) { + message.error(res.err); + } else { + message.success(t('common:success.delete')); + } + }) + .catch((err) => { + message.error(err?.message || 'Delete failed'); + return Promise.reject(err); + });As per coding guidelines, "Async requests must include 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/warning/subscribe/ListNG.tsx` around lines 311 - 318, The delete handler is calling deleteSubscribes without catching promise rejections and it shows a success toast even when res.err exists; update the call to handle errors and toast severity: call deleteSubscribes(...).then(res => { if (res.err) { message.error(res.err); } else { message.success(t('common:success.delete')); refreshList(); } }).catch(err => { message.error(err.message || String(err)); }); or convert to async/await inside the component method (try { const res = await deleteSubscribes(...); if (res.err) { message.error(res.err); } else { message.success(t('common:success.delete')); refreshList(); } } catch (err) { message.error(err.message || String(err)); }) so that deleteSubscribes, refreshList, message and t are used as shown and refreshList is only called on success.src/pages/dashboard/List/index.tsx (1)
340-347:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit error handling for row async actions.
cloneDashboard,getDashboard, andremoveDashboardsare awaited withouttry/catch(or.catch), so failure paths currently have no user feedback and can leave the row state ambiguous. Please handle rejection paths consistently with existing toast/error patterns.As per coding guidelines, "Async requests must include error handling (
try/catch,.catch,onError, etc.) consistent with existing patterns".Also applies to: 360-365, 377-384
🤖 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/dashboard/List/index.tsx` around lines 340 - 347, Wrap each awaited row-level async call (cloneDashboard, getDashboard, removeDashboards) inside try/catch blocks in the Modal.confirm onOk handlers and any inline onClick async handlers so failures show user feedback and keep UI state consistent; on success keep the existing message.success and setRefreshKey calls, and on error call message.error (including the caught error message or a translated failure key) and ensure any optimistic/disabled row state is reverted in the catch; update the handlers where cloneDashboard, getDashboard, and removeDashboards are awaited to follow this pattern.src/pages/user/users.tsx (1)
170-173:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
deleteUserfailure explicitly in confirmonOk.The delete path currently has success-only handling. Please add
try/catch(or.catch) and propagate rejection on failure.Proposed fix
- onOk: () => { - deleteUser(record.id).then((_) => { - message.success(t('common:success.delete')); - handleClose(); - }); - }, + onOk: async () => { + try { + await deleteUser(record.id); + message.success(t('common:success.delete')); + handleClose(); + } catch (err) { + return Promise.reject(err); + } + },As per coding guidelines, "Async requests must include 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/user/users.tsx` around lines 170 - 173, The confirm onOk callback currently calls deleteUser(record.id) with only success handling; update the onOk to handle failures by awaiting deleteUser inside a try/catch (or adding a .catch) so errors are caught, reported (e.g., message.error with the error message), and the rejection is propagated if needed; specifically modify the block around deleteUser(record.id) to wrap the call in try { await deleteUser(record.id); message.success(t('common:success.delete')); handleClose(); } catch (err) { message.error(err?.message || t('common:error.delete')); throw err; } so callers receive the failure.
🟡 Minor comments (2)
src/pages/warning/shield/index.tsx-85-85 (1)
85-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the hardcoded category label.
Line 85 uses a hardcoded
'Host', which bypasses i18n and can show mixed-language UI.🤖 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/index.tsx` at line 85, The code hardcodes 'Host' for record.cate === 'host', bypassing i18n; replace that branch with a localized label call (e.g. use getCateDisplayLabel('host', i18n.language) or the appropriate i18n key via t()) so cateLabel always comes from the localization function; update the ternary at const cateLabel = record.cate === 'host' ? 'Host' : getCateDisplayLabel(cate, i18n.language) to call getCateDisplayLabel or i18n for the 'host' case using getCateDisplayLabel, record.cate, and i18n.language to locate the change.src/theme/default.less-277-277 (1)
277-277:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stylelint keyword casing (
currentcolor).
background-color: currentColor;violatesvalue-keyword-casein this repo’s stylelint config.Suggested fix
- background-color: currentColor; + background-color: currentcolor;🤖 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/theme/default.less` at line 277, The background-color declaration uses the incorrect casing "currentColor" which triggers the stylelint value-keyword-case rule; change the CSS value for the background-color property to the lowercase keyword "currentcolor" (i.e., update the background-color: currentColor; occurrence to background-color: currentcolor;) so it conforms with stylelint.
🧹 Nitpick comments (2)
src/components/TableTags/index.tsx (1)
17-17: ⚡ Quick winRemove
anyfrom shared TableTags generics/helpers.Using
anyin this shared component weakens type guarantees and makes consumer mistakes easier to miss.As per coding guidelines, "Component Props must use interface to explicitly declare types, avoid any".
Also applies to: 30-38, 48-48
🤖 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/components/TableTags/index.tsx` at line 17, The Props generic currently uses `any` (interface Props<T = any>) which weakens type safety; change the default generic to a stricter type such as `T = Record<string, unknown>` (or `unknown` if appropriate) and update all related helper/type definitions to use that generic instead of `any` (the other occurrences noted around the helper/type declarations at ~lines 30-38 and the usage at ~line 48). Ensure functions and types inside this file (e.g., the TableTags component props, any tag-render or key-extraction helpers) accept `T` and carry the constraint through return types and parameters so consumers get proper type checking, and run the typechecker to fix any resulting call-site annotations.src/App.less (1)
384-426: ⚡ Quick winDeduplicate sorter overrides; keep them in one theme location.
This sorter block is duplicated with
src/theme/default.lessin the same PR. Keeping both will make future style changes error-prone due to precedence/drift.🤖 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/App.less` around lines 384 - 426, This file duplicates the table sorter CSS already defined in theme/default.less; remove the duplicated block in src/App.less (the .ant-table-column-sorter rules including .ant-table-column-sorter-up, .ant-table-column-sorter-down and the .ant-table-column-has-sorters override) and keep a single canonical definition in theme/default.less, or alternatively move any App-specific deltas into theme/default.less and ensure App imports the theme so styles remain centralized and not drift across files.
🤖 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/components/TableTags/index.tsx`:
- Around line 80-86: The click handler currently always calls e.stopPropagation,
which blocks row clicks even for non-interactive tags; change it so
stopPropagation is only called when the tag is actually interactive—i.e., when
the item has an onClick handler (the _.isObject && 'onClick' in (item as any)
check) or when the component-level onTagClick exists and is intended to consume
the event—then invoke (item as any).onClick?.() and onTagClick?.(item, index) as
before; otherwise do not call e.stopPropagation so table row click/selection
still works.
In `@src/pages/aiConfig/agents/pages/List.tsx`:
- Line 104: The column render callback uses the wrong parameter signature and
the delete button checks the wrong property: change the renderer from render:
(record) => ... to render: (value, record) => ... and use record.id from the
second parameter (not the first); also replace any checks of record.enable with
record.enabled (e.g., in the Delete button disable condition inside the render
for the actions column) so the delete button is disabled when the agent's
enabled flag is true. Ensure you update the render references in the actions
column where record.id and record.enabled are used.
In `@src/pages/aiConfig/llmConfigs/pages/List.tsx`:
- Line 96: The operations column's Table column uses a render function with the
wrong signature so the row data is undefined; update the render handler in
List.tsx to accept the ignored first param and the row record as the second
param (change render: (record) => to render: (_, record) =>) so uses of
record.id and record.enabled inside the render (operations column) reference the
actual row object.
In `@src/pages/aiConfig/mcpServers/pages/List.tsx`:
- Line 81: The operations column's render callback currently uses a single
parameter signature "render: (record) =>" which captures the cell value instead
of the row object, causing accesses to record.id and record.enabled to fail;
change the render signature to accept two parameters "render: (_, record) =>"
(matching the enabled column and the rest of the codebase) so the second
argument is the row record object, then use record.id and record.enabled as
before within that callback.
In `@src/pages/alertCurEvent/pages/List/AlertTable.tsx`:
- Line 190: The operations column's render function currently uses
render(record) so the first parameter is actually the cell text, not the row;
update the render signature to render(_, record) to receive the row object and
fix accesses to record.rule_prod, record.group_id, record.id, etc. Locate the
render method in the AlertTable column definition (the operations column's
render) and change its parameters to (_, record) and update any internal
references if needed to use that record variable consistently (matching the
pattern used in src/pages/user/users.tsx).
In `@src/pages/alertRules/List/ListNG.tsx`:
- Line 274: The render callback for the operations column is using the wrong
parameter signature; change the function assigned to render (currently declared
as render: (record: any) => ...) to accept the Ant Design table parameters so
the row data is the second arg — i.e., implement render: (_: unknown, record:
AlertRuleType<any>) => { ... } — and then update any usages inside that callback
that access record.id, record.cate, etc., to use the newly-typed record
parameter (ensure AlertRuleType is imported/available).
In `@src/pages/datasource/components/TableSource/index.tsx`:
- Around line 208-216: The confirm handlers for status toggle (using
updateDataSourceStatus) and the delete action currently only handle success
paths and may silently fail; wrap the async calls in a promise .catch (or
convert to async/await with try/catch) and on error call message.error with a
helpful message (e.g., t('error.somethingFailed') plus the error.message),
ensuring setRefresh((old)=>!old) and success message are only executed on
success; apply the same pattern to the delete handler (e.g.,
deleteDataSource/onConfirm) so both success and failure paths are handled and
users see an error when the request is rejected.
In `@src/pages/embeddedProduct/pages/List/index.tsx`:
- Around line 160-167: The Modal.confirm onOk currently calls
deleteEmbeddedProducts(...).then(...) but lacks failure handling; wrap the
delete call in a try/catch or attach a .catch to the promise returned by
deleteEmbeddedProducts (inside Modal.confirm onOk) and call message.error(...)
with a translated error message (use t(...) like other flows) when it fails,
ensuring you still avoid calling fetchData() or
eventBus.emit(EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED) on error; keep the existing
success behavior in the .then or try block and reference Modal.confirm,
deleteEmbeddedProducts, message.error, fetchData, eventBus.emit, and
EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED when making the change.
In `@src/pages/eventPipeline/pages/List/index.tsx`:
- Around line 278-284: The Modal.confirm onOk currently calls
deleteItems([item.id]) and only handles the success path; add error handling by
either awaiting deleteItems in an async onOk or appending .catch(...) to the
promise so failures are handled—on success call featchData(), on failure show a
user-facing error (e.g. message.error or Modal.error) including the error
message; update the Modal.confirm onOk block surrounding deleteItems,
featchData, and item.id to ensure failures are caught and reported.
In `@src/pages/log/IndexPatterns/index.tsx`:
- Around line 170-177: The save/delete async calls in the new menu path (e.g.,
the putESIndexPattern(...) call that updates record.id with fields_format built
from newFieldConfig and name, and the corresponding delete request in the same
menu) lack error handling; add .catch (or convert to async/await with try/catch)
to both requests so failures show user feedback and log the error. Specifically,
update the promise chain on putESIndexPattern(record.id, {...}) to handle errors
(log the error and call message.error(...) with a localized message and/or
err.message) and apply the same pattern to the delete action in the same menu
path so fetchData is only relied on after success and errors are surfaced to the
user.
In `@src/pages/notificationChannels/pages/List/index.tsx`:
- Around line 205-213: The delete confirmation flow calls
deleteItems([record.id]) but only handles success; modify the Modal.confirm onOk
handler to attach a .catch(...) to deleteItems so failures are handled—show a
user-visible error via message.error (including the caught error message or a
translation key) and avoid calling fetchData on failure; ensure the .then(...)
remains for success (message.success + fetchData) and that the error path logs
or surfaces the error consistent with other pages.
In `@src/pages/notificationChannels/pages/ListNG/index.tsx`:
- Around line 320-327: The delete flow opened by Modal.confirm calls
deleteItems([record.id]) but only handles success; add failure handling by
chaining a .catch(...) (or convert to async/await with try/catch) on the
deleteItems promise and show an error notification (e.g., message.error with a
translated string via t(...)) and ensure any cleanup/state refresh (run) only
occurs on success. Update the Modal.confirm onOk handler that currently calls
deleteItems([record.id]).then(() => { message.success(...); run(); }) to include
.catch(err => { message.error(t('common:failure.delete') || err.message); /*
optional: log err */ }) so request failures provide user feedback.
In `@src/pages/notificationRules/pages/List.tsx`:
- Around line 232-239: The delete handler invoked inside Modal.confirm currently
calls deleteItems([record.id]).then(() => { fetchData(); }) with no error
handling; update this to attach a .catch (or use async/await with try/catch) to
surface failures to the user and avoid silent failures—catch the error from
deleteItems (referencing Modal.confirm, deleteItems, fetchData, and record.id)
and notify the user (e.g., via message.error or the existing error reporting
utility) and optionally stop/rollback UI state, while preserving the success
path that calls fetchData().
- Around line 217-219: The edit link always builds the path as
`/${NS}/edit/${record.id}` but must respect the same IS_PLUS branch used
elsewhere (see the branching at line where IS_PLUS is checked); update the
TableActionLink for editing (the JSX using TableActionLink, NS and record.id) to
construct the pathname conditionally based on IS_PLUS so PLUS mode uses the
PLUS-specific route and non-PLUS uses `/${NS}/edit/${record.id}`.
In `@src/pages/recordingRules/PageTable.tsx`:
- Around line 247-250: The deleteRecordingRule call in the row action only
handles success; wrap the deletion in proper error handling (either convert to
async/await with try/catch or append .catch) on the
deleteRecordingRule([record.id], record.group_id) invocation, call
message.error(t('common:failed.delete') or a similar localized message including
the error.message for user feedback, ensure refreshList() is only called on
success, and also log the error (console.error or a logger) for debugging;
update the row action code that currently calls deleteRecordingRule(...) so
failures are surfaced to the user and not silently ignored.
In `@src/pages/taskTpl/index.tsx`:
- Around line 193-201: Modal.confirm's onOk currently returns request(...) but
lacks failure handling; update the onOk handler in the Modal.confirm block (the
function passed to onOk) that calls
request(`${api.tasktpl(record.group_id)}/${record.id}`, { method: 'DELETE' }) so
that errors are caught and handled—use async/try-catch (or append .catch) to
call message.error(...) with a descriptive message, ensure refresh() and
message.success(...) only run on success, and rethrow or return a rejected
promise on failure so the modal's promise flow receives the error.
In `@src/pages/user/business.tsx`:
- Around line 98-101: The confirm onOk currently calls
deleteBusinessTeamMember(...).then(...) but lacks error handling; update the
onOk handler in the confirm flow to await deleteBusinessTeamMember(teamId,
params) inside a try/catch (or add a .catch) so failures are handled: on success
call message.success(...) and getTeamList(), on error call message.error(...)
(or use the existing error reporting utility) and do not call getTeamList;
ensure you reference the existing deleteBusinessTeamMember, message.success,
message.error (or error handler) and getTeamList symbols when making the change.
In `@src/pages/user/groups.tsx`:
- Around line 135-138: The confirm flow currently calls deleteMember(teamId,
params).then(...) without handling failures; update the onOk handler that
invokes deleteMember to use async/await or attach .catch so errors are handled
and propagated: await deleteMember(teamId, params) inside a try/catch (or add
.catch) and on success call message.success(t('common:success.delete')) and
handleClose('updateMember'), but on failure call message.error(...) (or process
the error) and rethrow or return a rejected promise so the caller knows deletion
failed; refer to the deleteMember function invocation and the
handleClose/message.success calls to locate where to add the try/catch/.catch
and propagate the rejection.
In `@src/pages/variableConfigs/index.tsx`:
- Around line 170-176: The Modal.confirm onOk handler currently calls
deleteVariableConfigs(record.id) without handling rejections; update the onOk
callback to await or return deleteVariableConfigs(record.id) and add error
handling so failures show feedback and don't leave unhandled rejections—for
example use .then(...).catch(err => message.error(t('common:fail.delete') + ': '
+ (err?.message || err))) or try/catch if converting to async, and ensure
fetchData() only runs on success; reference Modal.confirm, onOk,
deleteVariableConfigs, fetchData, message.error and t to locate and modify the
code.
In `@src/pages/warning/shield/index.tsx`:
- Around line 303-310: The deleteShields call is missing rejection handling and
incorrectly uses message.success for error messages; update the logic around
deleteShields({ ids: [record.id] }, record.group_id) (and the surrounding
refreshList usage) to await or attach a .catch handler, ensure you check res.err
and call message.error(res.err) (or message.error with a sensible fallback) when
there is an error, and only call message.success(t('common:success.delete')) on
successful deletion; also ensure refreshList is only invoked after a successful
delete or is placed appropriately in the finally block so failures are handled
consistently.
---
Outside diff comments:
In `@src/pages/dashboard/List/index.tsx`:
- Around line 340-347: Wrap each awaited row-level async call (cloneDashboard,
getDashboard, removeDashboards) inside try/catch blocks in the Modal.confirm
onOk handlers and any inline onClick async handlers so failures show user
feedback and keep UI state consistent; on success keep the existing
message.success and setRefreshKey calls, and on error call message.error
(including the caught error message or a translated failure key) and ensure any
optimistic/disabled row state is reverted in the catch; update the handlers
where cloneDashboard, getDashboard, and removeDashboards are awaited to follow
this pattern.
In `@src/pages/user/users.tsx`:
- Around line 170-173: The confirm onOk callback currently calls
deleteUser(record.id) with only success handling; update the onOk to handle
failures by awaiting deleteUser inside a try/catch (or adding a .catch) so
errors are caught, reported (e.g., message.error with the error message), and
the rejection is propagated if needed; specifically modify the block around
deleteUser(record.id) to wrap the call in try { await deleteUser(record.id);
message.success(t('common:success.delete')); handleClose(); } catch (err) {
message.error(err?.message || t('common:error.delete')); throw err; } so callers
receive the failure.
In `@src/pages/warning/subscribe/ListNG.tsx`:
- Around line 311-318: The delete handler is calling deleteSubscribes without
catching promise rejections and it shows a success toast even when res.err
exists; update the call to handle errors and toast severity: call
deleteSubscribes(...).then(res => { if (res.err) { message.error(res.err); }
else { message.success(t('common:success.delete')); refreshList(); }
}).catch(err => { message.error(err.message || String(err)); }); or convert to
async/await inside the component method (try { const res = await
deleteSubscribes(...); if (res.err) { message.error(res.err); } else {
message.success(t('common:success.delete')); refreshList(); } } catch (err) {
message.error(err.message || String(err)); }) so that deleteSubscribes,
refreshList, message and t are used as shown and refreshList is only called on
success.
---
Minor comments:
In `@src/pages/warning/shield/index.tsx`:
- Line 85: The code hardcodes 'Host' for record.cate === 'host', bypassing i18n;
replace that branch with a localized label call (e.g. use
getCateDisplayLabel('host', i18n.language) or the appropriate i18n key via t())
so cateLabel always comes from the localization function; update the ternary at
const cateLabel = record.cate === 'host' ? 'Host' : getCateDisplayLabel(cate,
i18n.language) to call getCateDisplayLabel or i18n for the 'host' case using
getCateDisplayLabel, record.cate, and i18n.language to locate the change.
In `@src/theme/default.less`:
- Line 277: The background-color declaration uses the incorrect casing
"currentColor" which triggers the stylelint value-keyword-case rule; change the
CSS value for the background-color property to the lowercase keyword
"currentcolor" (i.e., update the background-color: currentColor; occurrence to
background-color: currentcolor;) so it conforms with stylelint.
---
Nitpick comments:
In `@src/App.less`:
- Around line 384-426: This file duplicates the table sorter CSS already defined
in theme/default.less; remove the duplicated block in src/App.less (the
.ant-table-column-sorter rules including .ant-table-column-sorter-up,
.ant-table-column-sorter-down and the .ant-table-column-has-sorters override)
and keep a single canonical definition in theme/default.less, or alternatively
move any App-specific deltas into theme/default.less and ensure App imports the
theme so styles remain centralized and not drift across files.
In `@src/components/TableTags/index.tsx`:
- Line 17: The Props generic currently uses `any` (interface Props<T = any>)
which weakens type safety; change the default generic to a stricter type such as
`T = Record<string, unknown>` (or `unknown` if appropriate) and update all
related helper/type definitions to use that generic instead of `any` (the other
occurrences noted around the helper/type declarations at ~lines 30-38 and the
usage at ~line 48). Ensure functions and types inside this file (e.g., the
TableTags component props, any tag-render or key-extraction helpers) accept `T`
and carry the constraint through return types and parameters so consumers get
proper type checking, and run the typechecker to fix any resulting call-site
annotations.
🪄 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: efca9371-f961-475b-90a7-14b218b888b9
📒 Files selected for processing (47)
scripts/generate_antd_dark_less.jsscripts/generate_antd_gold_less.jssrc/App.lesssrc/components/TableActionDropdown/index.tsxsrc/components/TableTags/index.tsxsrc/components/TableTags/style.lesssrc/components/Tags/index.tsxsrc/main.tsxsrc/pages/aiConfig/agents/pages/List.tsxsrc/pages/aiConfig/llmConfigs/pages/List.tsxsrc/pages/aiConfig/mcpServers/pages/List.tsxsrc/pages/aiConfig/skills/pages/ResourcesTable.tsxsrc/pages/alertCurEvent/pages/List/AlertTable.tsxsrc/pages/alertRules/List/ListNG.tsxsrc/pages/alertRules/List/constants.tssrc/pages/builtInComponents/AlertRules/index.tsxsrc/pages/builtInComponents/CollectTpls/index.tsxsrc/pages/builtInComponents/Dashboards/index.tsxsrc/pages/builtInComponents/Metrics/index.tsxsrc/pages/dashboard/List/index.tsxsrc/pages/datasource/components/TableSource/index.tsxsrc/pages/embeddedProduct/pages/List/index.tsxsrc/pages/event/Table.tsxsrc/pages/event/card.tsxsrc/pages/eventPipeline/pages/List/index.tsxsrc/pages/historyEvents/ListNG/index.tsxsrc/pages/hosts/pages/List/List.tsxsrc/pages/hosts/pages/List/Tags.tsxsrc/pages/log/IndexPatterns/index.tsxsrc/pages/metricsBuiltin/List.tsxsrc/pages/notificationChannels/pages/List/index.tsxsrc/pages/notificationChannels/pages/ListNG/index.tsxsrc/pages/notificationRules/pages/List.tsxsrc/pages/recordingRules/PageTable.tsxsrc/pages/targets/List.tsxsrc/pages/task/index.tsxsrc/pages/taskTpl/index.tsxsrc/pages/user/business.tsxsrc/pages/user/component/Tags/index.tsxsrc/pages/user/constants.tssrc/pages/user/groups.tsxsrc/pages/user/users.tsxsrc/pages/variableConfigs/index.tsxsrc/pages/warning/shield/index.tsxsrc/pages/warning/subscribe/ListNG.tsxsrc/theme/default.lessvite.config.ts
💤 Files with no reviewable changes (4)
- src/pages/hosts/pages/List/Tags.tsx
- src/pages/user/component/Tags/index.tsx
- src/pages/user/constants.ts
- src/components/Tags/index.tsx
Comment on lines
+80
to
+86
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| if (_.isObject(item) && 'onClick' in (item as any)) { | ||
| (item as any).onClick?.(); | ||
| } | ||
| onTagClick?.(item, index); | ||
| }} |
There was a problem hiding this comment.
Avoid blocking row click for non-interactive tags.
stopPropagation() runs on every tag click, including non-clickable tags, which can break row click/selection behaviors in tables.
Suggested fix
- <span
+ <span
className={`fc-table-tag${clickable ? ' fc-table-tag-clickable' : ''}`}
style={style}
- onClick={(e) => {
- e.stopPropagation();
- if (_.isObject(item) && 'onClick' in (item as any)) {
- (item as any).onClick?.();
- }
- onTagClick?.(item, index);
- }}
+ onClick={
+ clickable
+ ? (e) => {
+ e.stopPropagation();
+ if (_.isObject(item) && 'onClick' in (item as any)) {
+ (item as any).onClick?.();
+ }
+ onTagClick?.(item, index);
+ }
+ : 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/components/TableTags/index.tsx` around lines 80 - 86, The click handler
currently always calls e.stopPropagation, which blocks row clicks even for
non-interactive tags; change it so stopPropagation is only called when the tag
is actually interactive—i.e., when the item has an onClick handler (the
_.isObject && 'onClick' in (item as any) check) or when the component-level
onTagClick exists and is intended to consume the event—then invoke (item as
any).onClick?.() and onTagClick?.(item, index) as before; otherwise do not call
e.stopPropagation so table row click/selection still works.
| width: 100, | ||
| width: 64, | ||
| fixed: 'right' as const, | ||
| render: (record) => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 150 src/pages/aiConfig/agents/pages/List.tsx | tail -n 60Repository: n9e/fe
Length of output: 2858
🏁 Script executed:
# Search for usage patterns of the enable/enabled property in this file
rg -n "\.enable" src/pages/aiConfig/agents/pages/List.tsxRepository: n9e/fe
Length of output: 247
🏁 Script executed:
# Check the data structure or interfaces used for the table
rg -n "interface.*Agent|type.*Agent" src/pages/aiConfig/agents/pages/List.tsx -A 5Repository: n9e/fe
Length of output: 32
🏁 Script executed:
# Search for other render functions in the same file to see the pattern
rg -n "render:\s*\(" src/pages/aiConfig/agents/pages/List.tsx -A 2Repository: n9e/fe
Length of output: 291
🏁 Script executed:
# Search for the data source and type definitions
rg -n "type.*Agent|interface.*Agent|enabled|enable" src/pages/aiConfig/agents/pages/List.tsx -B 2 -A 2Repository: n9e/fe
Length of output: 1121
🏁 Script executed:
# Check the API call or data structure
rg -n "data\?" src/pages/aiConfig/agents/pages/List.tsx | head -20Repository: n9e/fe
Length of output: 32
🏁 Script executed:
# Look for service/API files that might define the Agent type
fd -e ts -e tsx "agent" src/pages/aiConfig/agents --type fRepository: n9e/fe
Length of output: 32
🏁 Script executed:
# Search for where this list data comes from
rg -n "getList\|fetchList\|\.get\(" src/pages/aiConfig/agents/pages/List.tsx | head -10Repository: n9e/fe
Length of output: 32
🏁 Script executed:
# Search for Agent type definition across the codebase
fd -e ts -e tsx "service\|api" src/pages/aiConfig/agents --type f | head -10Repository: n9e/fe
Length of output: 32
🏁 Script executed:
# Look at the types being used in the file
head -30 src/pages/aiConfig/agents/pages/List.tsxRepository: n9e/fe
Length of output: 1108
🏁 Script executed:
# Search for imports to understand the data structure
rg -n "import.*Agent" src/pages/aiConfig/agents/pages/List.tsxRepository: n9e/fe
Length of output: 32
🏁 Script executed:
# Verify the AntD Table Column.render API signature in documentation
web_search "antd Table Column render callback signature value record index"Repository: n9e/fe
Length of output: 101
🏁 Script executed:
# Check if there are other files in the same directory using similar patterns
rg -n "render:\s*\(" src/pages/aiConfig --type ts --type tsx -A 1 | head -30Repository: n9e/fe
Length of output: 78
🌐 Web query:
antd Table Column render callback API signature parameters
💡 Result:
In the Ant Design (antd) Table component, the render callback function for a column is defined with the following API signature: (value: V, record: T, index: number) => ReactNode The parameters are defined as follows: 1. value (V): The value of the current cell, typically derived from the dataIndex property of the column [1]. 2. record (T): The entire data object for the current row [1][2]. This is useful when you need access to other fields in the same row to determine the rendering logic [3][4]. 3. index (number): The row number (index) of the current record within the data source [1]. The function must return a valid ReactNode, which allows you to render custom components, HTML elements, or formatted text within the table cell [1][4]. Common usage patterns often involve using the underscore () convention for unused parameters, such as render: (, record) => Delete {record.id}, when the cell's specific value is not needed [5][4].
Citations:
- 1: https://ant.design/components/table/
- 2: https://medium.com/@hisi480284856/understanding-the-render-function-in-react-tables-a-beginners-guide-ff96eb2daf7b
- 3: https://stackoverflow.com/questions/69767578/what-antd-column-render-parameters-stand-for
- 4: https://app.studyraid.com/en/read/12423/401173/custom-table-cell-rendering
- 5: https://ant.design/components/table.md
🏁 Script executed:
# Search for Agent type definition in services
fd "service" src/pages/aiConfig/agents --type fRepository: n9e/fe
Length of output: 88
🏁 Script executed:
# Look at the services file
find src/pages/aiConfig/agents -name "*service*" -o -name "*api*" 2>/dev/nullRepository: n9e/fe
Length of output: 88
🏁 Script executed:
# Check what putItem is called with to infer the type
rg -n "putItem\|deleteItem\|getList" src/pages/aiConfig/agents/pages/List.tsx -B 2 -A 2Repository: n9e/fe
Length of output: 32
🏁 Script executed:
# Look at the services file to understand the data structure
cat src/pages/aiConfig/agents/services.tsRepository: n9e/fe
Length of output: 1097
🏁 Script executed:
# Check the types file to see if the property is 'enable' or 'enabled'
cat src/pages/aiConfig/agents/types.tsRepository: n9e/fe
Length of output: 559
Fix operations render callback signature and enabled property key.
Line 104's render: (record) => ... is incorrect—the first parameter should be the cell value, second should be the row record. This makes record.id unsafe. Additionally, lines 126 and 130 check record.enable instead of the correct property record.enabled, so the delete button is never disabled when the agent is enabled.
Suggested fix
- render: (record) => {
+ render: (_, record) => {
return (
<Dropdown
...
- <Tooltip title={record.enable === true ? t('cannot_delete_when_enabled') : undefined}>
+ <Tooltip title={record.enabled === true ? t('cannot_delete_when_enabled') : undefined}>
<TableActionButton
actionIcon='delete'
danger
- disabled={record.enable === true}
+ disabled={record.enabled === true}📝 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
| render: (record) => { | |
| render: (_, record) => { | |
| return ( | |
| <Dropdown | |
| menu={{ | |
| items: [ | |
| { | |
| key: 'edit', | |
| label: t('edit'), | |
| onClick: () => { | |
| setSelectedRecord(record); | |
| setDrawerOpen(true); | |
| }, | |
| }, | |
| { | |
| key: 'delete', | |
| label: ( | |
| <Tooltip title={record.enabled === true ? t('cannot_delete_when_enabled') : undefined}> | |
| <TableActionButton | |
| actionIcon='delete' | |
| danger | |
| disabled={record.enabled === true} | |
| onClick={() => handleDelete(record.id)} | |
| /> | |
| </Tooltip> | |
| ), | |
| disabled: record.enabled === true, | |
| }, | |
| ], | |
| }} | |
| > | |
| <Button type='text' icon={<EllipsisOutlined />} /> | |
| </Dropdown> | |
| ); | |
| }, |
🤖 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/agents/pages/List.tsx` at line 104, The column render
callback uses the wrong parameter signature and the delete button checks the
wrong property: change the renderer from render: (record) => ... to render:
(value, record) => ... and use record.id from the second parameter (not the
first); also replace any checks of record.enable with record.enabled (e.g., in
the Delete button disable condition inside the render for the actions column) so
the delete button is disabled when the agent's enabled flag is true. Ensure you
update the render references in the actions column where record.id and
record.enabled are used.
| width: 100, | ||
| width: 64, | ||
| fixed: 'right' as const, | ||
| render: (record) => ( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file around line 81 to see the context
cat -n src/pages/aiConfig/mcpServers/pages/List.tsx | sed -n '70,120p'Repository: n9e/fe
Length of output: 2801
🏁 Script executed:
# Search for other render callbacks in Table columns to see patterns used
rg "render:\s*\(" src/pages/aiConfig/mcpServers/pages/List.tsx -A 2 -B 1Repository: n9e/fe
Length of output: 366
🏁 Script executed:
# Check AntD version to confirm render signature expectations
rg "antd" package.json -A 1Repository: n9e/fe
Length of output: 265
🏁 Script executed:
# Check if there are other operations columns in other files to see standard pattern
rg "render:.*record" src/pages -B 2 -A 5 --type tsx | head -50Repository: n9e/fe
Length of output: 78
🏁 Script executed:
# Let's check the pattern in other table files in the project
rg "render:\s*\(\w+" src/pages -B 1 -A 3 --type-add 'tsx:*.{ts,tsx}' | head -60Repository: n9e/fe
Length of output: 3758
🏁 Script executed:
# Check the actual implementation to understand if this might work accidentally
# by looking at what the first cell value would be for the operations column
cat -n src/pages/aiConfig/mcpServers/pages/List.tsx | sed -n '55,85p'Repository: n9e/fe
Length of output: 1443
Operations column render callback has incorrect parameter signature.
Line 81 uses render: (record) => which only captures the first parameter (cell value). The callback should accept two parameters (_, record) to correctly receive the row record object as the second parameter. Without this fix, accessing record.id and record.enabled will fail at runtime. This pattern is inconsistent with the enabled column at line 61 in the same file and the rest of the codebase.
🤖 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/mcpServers/pages/List.tsx` at line 81, The operations
column's render callback currently uses a single parameter signature "render:
(record) =>" which captures the cell value instead of the row object, causing
accesses to record.id and record.enabled to fail; change the render signature to
accept two parameters "render: (_, record) =>" (matching the enabled column and
the rest of the codebase) so the second argument is the row record object, then
use record.id and record.enabled as before within that callback.
Comment on lines
+193
to
+201
| Modal.confirm({ | ||
| title: t('common:confirm.delete'), | ||
| onOk: () => { | ||
| return request(`${api.tasktpl(record.group_id)}/${record.id}`, { | ||
| method: 'DELETE', | ||
| }).then(() => { | ||
| message.success(t('msg.delete.success')); | ||
| refresh(); | ||
| }); |
There was a problem hiding this comment.
Add explicit failure handling for template deletion in Modal.confirm.
onOk only handles success. Add try/catch (or .catch) so request failures are handled deterministically and propagated correctly to the modal promise flow.
Proposed fix
- onOk: () => {
- return request(`${api.tasktpl(record.group_id)}/${record.id}`, {
- method: 'DELETE',
- }).then(() => {
- message.success(t('msg.delete.success'));
- refresh();
- });
- },
+ onOk: async () => {
+ try {
+ await request(`${api.tasktpl(record.group_id)}/${record.id}`, {
+ method: 'DELETE',
+ });
+ message.success(t('msg.delete.success'));
+ refresh();
+ } catch (err) {
+ return Promise.reject(err);
+ }
+ },As per coding guidelines, "Async requests must include error handling (try/catch, .catch, onError, etc.) consistent with existing patterns".
📝 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
| Modal.confirm({ | |
| title: t('common:confirm.delete'), | |
| onOk: () => { | |
| return request(`${api.tasktpl(record.group_id)}/${record.id}`, { | |
| method: 'DELETE', | |
| }).then(() => { | |
| message.success(t('msg.delete.success')); | |
| refresh(); | |
| }); | |
| Modal.confirm({ | |
| title: t('common:confirm.delete'), | |
| onOk: async () => { | |
| try { | |
| await request(`${api.tasktpl(record.group_id)}/${record.id}`, { | |
| method: 'DELETE', | |
| }); | |
| message.success(t('msg.delete.success')); | |
| refresh(); | |
| } catch (err) { | |
| return Promise.reject(err); | |
| } | |
| }, |
🤖 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/taskTpl/index.tsx` around lines 193 - 201, Modal.confirm's onOk
currently returns request(...) but lacks failure handling; update the onOk
handler in the Modal.confirm block (the function passed to onOk) that calls
request(`${api.tasktpl(record.group_id)}/${record.id}`, { method: 'DELETE' }) so
that errors are caught and handled—use async/try-catch (or append .catch) to
call message.error(...) with a descriptive message, ensure refresh() and
message.success(...) only run on success, and rethrow or return a rejected
promise on failure so the modal's promise flow receives the error.
Comment on lines
+98
to
+101
| deleteBusinessTeamMember(teamId, params).then(() => { | ||
| message.success(t('common:success.delete')); | ||
| getTeamList(); | ||
| }); |
There was a problem hiding this comment.
Handle deleteBusinessTeamMember failures inside confirm onOk.
This new async branch only handles success. Add try/catch (or .catch) to keep failure behavior explicit and predictable.
Proposed fix
- onOk: () => {
- deleteBusinessTeamMember(teamId, params).then(() => {
- message.success(t('common:success.delete'));
- getTeamList();
- });
- },
+ onOk: async () => {
+ try {
+ await deleteBusinessTeamMember(teamId, params);
+ message.success(t('common:success.delete'));
+ getTeamList();
+ } catch (err) {
+ return Promise.reject(err);
+ }
+ },As per coding guidelines, "Async requests must include 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/user/business.tsx` around lines 98 - 101, The confirm onOk
currently calls deleteBusinessTeamMember(...).then(...) but lacks error
handling; update the onOk handler in the confirm flow to await
deleteBusinessTeamMember(teamId, params) inside a try/catch (or add a .catch) so
failures are handled: on success call message.success(...) and getTeamList(), on
error call message.error(...) (or use the existing error reporting utility) and
do not call getTeamList; ensure you reference the existing
deleteBusinessTeamMember, message.success, message.error (or error handler) and
getTeamList symbols when making the change.
Comment on lines
+135
to
+138
| deleteMember(teamId, params).then((_) => { | ||
| message.success(t('common:success.delete')); | ||
| handleClose('updateMember'); | ||
| }); |
There was a problem hiding this comment.
Add explicit error handling for deleteMember in confirm flow.
The new delete path handles only success. Please add try/catch (or .catch) in onOk and propagate rejection when deletion fails.
Proposed fix
- onOk: () => {
- deleteMember(teamId, params).then((_) => {
- message.success(t('common:success.delete'));
- handleClose('updateMember');
- });
- },
+ onOk: async () => {
+ try {
+ await deleteMember(teamId, params);
+ message.success(t('common:success.delete'));
+ handleClose('updateMember');
+ } catch (err) {
+ return Promise.reject(err);
+ }
+ },As per coding guidelines, "Async requests must include error handling (try/catch, .catch, onError, etc.) consistent with existing patterns".
📝 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
| deleteMember(teamId, params).then((_) => { | |
| message.success(t('common:success.delete')); | |
| handleClose('updateMember'); | |
| }); | |
| onOk: async () => { | |
| try { | |
| await deleteMember(teamId, params); | |
| message.success(t('common:success.delete')); | |
| handleClose('updateMember'); | |
| } catch (err) { | |
| return Promise.reject(err); | |
| } | |
| }, |
🤖 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/user/groups.tsx` around lines 135 - 138, The confirm flow currently
calls deleteMember(teamId, params).then(...) without handling failures; update
the onOk handler that invokes deleteMember to use async/await or attach .catch
so errors are handled and propagated: await deleteMember(teamId, params) inside
a try/catch (or add .catch) and on success call
message.success(t('common:success.delete')) and handleClose('updateMember'), but
on failure call message.error(...) (or process the error) and rethrow or return
a rejected promise so the caller knows deletion failed; refer to the
deleteMember function invocation and the handleClose/message.success calls to
locate where to add the try/catch/.catch and propagate the rejection.
Comment on lines
+170
to
+176
| Modal.confirm({ | ||
| title: t('common:confirm.delete'), | ||
| onOk: () => { | ||
| deleteVariableConfigs(record.id).then(() => { | ||
| message.success(t('common:success.delete')); | ||
| fetchData(); | ||
| }); |
There was a problem hiding this comment.
Handle delete request failures in the confirm callback.
onOk only handles success. If the API rejects, users get no feedback and the failure path is unhandled.
Suggested fix
- onOk: () => {
- deleteVariableConfigs(record.id).then(() => {
- message.success(t('common:success.delete'));
- fetchData();
- });
- },
+ onOk: () => {
+ return deleteVariableConfigs(record.id)
+ .then(() => {
+ message.success(t('common:success.delete'));
+ fetchData();
+ })
+ .catch((err) => {
+ message.error(err?.message || 'Delete failed');
+ return Promise.reject(err);
+ });
+ },As per coding guidelines, "Async requests must include 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/variableConfigs/index.tsx` around lines 170 - 176, The
Modal.confirm onOk handler currently calls deleteVariableConfigs(record.id)
without handling rejections; update the onOk callback to await or return
deleteVariableConfigs(record.id) and add error handling so failures show
feedback and don't leave unhandled rejections—for example use
.then(...).catch(err => message.error(t('common:fail.delete') + ': ' +
(err?.message || err))) or try/catch if converting to async, and ensure
fetchData() only runs on success; reference Modal.confirm, onOk,
deleteVariableConfigs, fetchData, message.error and t to locate and modify the
code.
Comment on lines
+303
to
+310
| deleteShields({ ids: [record.id] }, record.group_id).then((res) => { | ||
| refreshList(); | ||
| if (res.err) { | ||
| message.success(res.err); | ||
| } else { | ||
| message.success(t('common:success.delete')); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Use error feedback for delete failures and handle rejected promises.
On Line 306, res.err is shown with message.success, and the request has no rejection handling.
Suggested fix
- deleteShields({ ids: [record.id] }, record.group_id).then((res) => {
- refreshList();
- if (res.err) {
- message.success(res.err);
- } else {
- message.success(t('common:success.delete'));
- }
- });
+ return deleteShields({ ids: [record.id] }, record.group_id)
+ .then((res) => {
+ refreshList();
+ if (res.err) {
+ message.error(res.err);
+ } else {
+ message.success(t('common:success.delete'));
+ }
+ })
+ .catch((err) => {
+ message.error(err?.message || 'Delete failed');
+ return Promise.reject(err);
+ });As per coding guidelines, "Async requests must include error handling (try/catch, .catch, onError, etc.) consistent with existing patterns".
📝 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
| deleteShields({ ids: [record.id] }, record.group_id).then((res) => { | |
| refreshList(); | |
| if (res.err) { | |
| message.success(res.err); | |
| } else { | |
| message.success(t('common:success.delete')); | |
| } | |
| }); | |
| return deleteShields({ ids: [record.id] }, record.group_id) | |
| .then((res) => { | |
| refreshList(); | |
| if (res.err) { | |
| message.error(res.err); | |
| } else { | |
| message.success(t('common:success.delete')); | |
| } | |
| }) | |
| .catch((err) => { | |
| message.error(err?.message || 'Delete failed'); | |
| return Promise.reject(err); | |
| }); |
🤖 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/index.tsx` around lines 303 - 310, The deleteShields
call is missing rejection handling and incorrectly uses message.success for
error messages; update the logic around deleteShields({ ids: [record.id] },
record.group_id) (and the surrounding refreshList usage) to await or attach a
.catch handler, ensure you check res.err and call message.error(res.err) (or
message.error with a sensible fallback) when there is an error, and only call
message.success(t('common:success.delete')) on successful deletion; also ensure
refreshList is only invoked after a successful delete or is placed appropriately
in the finally block so failures are handled consistently.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/App.less (1)
384-444: ⚡ Quick winDeduplicate table sorter overrides to a single theme source.
Line 384 through Line 444 duplicates global sorter/header override logic that is also defined in
src/theme/default.lessin this PR. Keeping both copies will drift and create precedence ambiguity; keep one canonical block (prefersrc/theme/default.less) and remove the duplicate here.As per coding guidelines, "Keep Less/CSS selectors as locally scoped as possible to avoid global style conflicts."
🤖 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/App.less` around lines 384 - 444, This file contains a duplicated Less block for table sorter/header overrides (look for selectors .ant-table-thead > tr > th, .ant-table-column-sorter, and .ant-table-column-has-sorters .ant-table-column-sorter-inner) that should be removed here and consolidated into the canonical theme source; delete the duplicated block from this file (the entire .ant-table-column-sorter/.ant-table-thead override section) and ensure only the single canonical implementation in the theme file remains, keeping selector behavior and SVG masks unchanged there to avoid precedence drift and global conflicts.src/theme/default.less (1)
270-270: ⚡ Quick winReplace hardcoded black with a theme token in danger hover color.
Line 270 mixes
var(--fc-fill-error)with#000, which introduces a magic color in theme styles. Use a theme variable for the mix target.Proposed fix
- color: color-mix(in srgb, var(--fc-fill-error) 82%, `#000`); + color: color-mix(in srgb, var(--fc-fill-error) 82%, var(--fc-text-1));As per coding guidelines, "Use color and theme-related values from
src/theme/variable.cssand existing theme system; avoid magic color values."🤖 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/theme/default.less` at line 270, The hover color currently mixes var(--fc-fill-error) with a hardcoded `#000` in the rule that sets "color: color-mix(in srgb, var(--fc-fill-error) 82%, `#000`);" — replace the magic "`#000`" with the appropriate theme token (for example var(--fc-fill-foreground) or another neutral/foreground token defined in src/theme/variable.css), update the color-mix call accordingly, and if no suitable token exists, add a descriptive token in variable.css and use that token here so the danger hover color uses only theme variables.
🤖 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/App.less`:
- Line 414: Replace the background-color value "currentColor" with the lowercase
"currentcolor" for the sorter icon color in App.less (i.e., change the
background-color: currentColor declaration to background-color: currentcolor) so
it conforms to the stylelint value-keyword-case rule.
In `@src/theme/default.less`:
- Line 293: Change the CSS keyword from the camelCase token "currentColor" to
the lowercased keyword expected by stylelint so the background-color value
conforms to the value-keyword-case rule; locate the background-color declaration
in src/theme/default.less (the line currently using background-color:
currentColor;) and replace the token with the lowercase form ("currentcolor") so
the linter no longer fails.
- Around line 113-133: The CSS hides interactive sorter/filter controls with
visibility:hidden which breaks keyboard access; update the rules for
.ant-table-column-sorter and .ant-table-filter-trigger to avoid
visibility:hidden (use only opacity and pointer events if needed) and add a
:focus-within selector in addition to :hover to reveal them (e.g., reveal on
&:hover and &:focus-within and keep the existing active selectors for
.ant-table-column-sorter-up.active/.ant-table-column-sorter-down.active/.ant-table-filter-trigger.active);
preserve the transition declarations so showing/hiding remains animated and
ensure any pointer-events or keyboard-focus styles allow keyboard users to reach
and operate the controls.
---
Nitpick comments:
In `@src/App.less`:
- Around line 384-444: This file contains a duplicated Less block for table
sorter/header overrides (look for selectors .ant-table-thead > tr > th,
.ant-table-column-sorter, and .ant-table-column-has-sorters
.ant-table-column-sorter-inner) that should be removed here and consolidated
into the canonical theme source; delete the duplicated block from this file (the
entire .ant-table-column-sorter/.ant-table-thead override section) and ensure
only the single canonical implementation in the theme file remains, keeping
selector behavior and SVG masks unchanged there to avoid precedence drift and
global conflicts.
In `@src/theme/default.less`:
- Line 270: The hover color currently mixes var(--fc-fill-error) with a
hardcoded `#000` in the rule that sets "color: color-mix(in srgb,
var(--fc-fill-error) 82%, `#000`);" — replace the magic "`#000`" with the
appropriate theme token (for example var(--fc-fill-foreground) or another
neutral/foreground token defined in src/theme/variable.css), update the
color-mix call accordingly, and if no suitable token exists, add a descriptive
token in variable.css and use that token here so the danger hover color uses
only theme variables.
🪄 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: 4a35d91c-9b87-4965-bb49-faae06d242be
📒 Files selected for processing (7)
scripts/generate_antd_dark_less.jsscripts/generate_antd_gold_less.jssrc/App.lesssrc/theme/antd.dark.lesssrc/theme/antd.light-gold.lesssrc/theme/default.lessvite.config.ts
✅ Files skipped from review due to trivial changes (1)
- vite.config.ts
| transition: all 0.2s; | ||
| width: 12px; | ||
| height: 12px; | ||
| background-color: currentColor; |
There was a problem hiding this comment.
Fix Stylelint keyword casing for sorter icon color.
Line 414 uses currentColor, which violates the configured value-keyword-case rule and will fail lint.
Proposed fix
- background-color: currentColor;
+ background-color: currentcolor;📝 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
| background-color: currentColor; | |
| background-color: currentcolor; |
🧰 Tools
🪛 Stylelint (17.11.0)
[error] 414-414: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🤖 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/App.less` at line 414, Replace the background-color value "currentColor"
with the lowercase "currentcolor" for the sorter icon color in App.less (i.e.,
change the background-color: currentColor declaration to background-color:
currentcolor) so it conforms to the stylelint value-keyword-case rule.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/alertCurEvent/pages/List/AlertTable.tsx (1)
305-306:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPage-size defaults are not aligned with the stated table design objective.
Line 305 still sets
defaultPageSize: 30, and Line 332 omits15frompageSizeOptions. If this table is in the rollout scope, this misses the intended default behavior.Suggested fix
- defaultPageSize: 30, + defaultPageSize: 15, ... - pageSizeOptions: ['30', '100', '200', '500'], + pageSizeOptions: ['15', '30', '100', '200', '500'],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/alertCurEvent/pages/List/AlertTable.tsx` around lines 305 - 306, The table pagination defaults are incorrect: in AlertTable update the pagination config so defaultPageSize is 15 (not 30) and ensure the pageSizeOptions array includes 15 (e.g., [..., 15, ...]) so the default matches the selectable options; locate the pagination object where defaultPageSize and pageSizeOptions are defined and change the values there (references: defaultPageSize, pageSizeOptions in AlertTable component).
♻️ Duplicate comments (3)
src/pages/alertCurEvent/pages/List/AlertTable.tsx (1)
195-195:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix operations column render signature to receive the row object.
Line 195 uses
render(record), but in antd Table this first argument is cell text/value. Accesses like Line 213/221/241 therefore risk reading from the wrong object at runtime. Userender(_, record).Suggested fix
- render(record) { + render(_, record) { return (Also applies to: 206-226, 241-244
🤖 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/alertCurEvent/pages/List/AlertTable.tsx` at line 195, The operations column renderer in AlertTable.tsx is using the wrong parameter signature (render(record)) so it receives the cell value instead of the row object; update the render functions for the operations column(s) to accept the value placeholder and the row object (e.g., render(_, record)) wherever used (notably the render at the current function around the operations column and the other render occurrences you flagged between lines ~206-226 and ~241-244) and then adjust any references that expect the full row (e.g., accesses to record.id, record.status, etc.) to use the second parameter (record).src/theme/default.less (2)
288-288:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix CSS keyword casing for sorter icon color.
Line 288 uses
currentColor, which failsvalue-keyword-casein the provided static analysis.Suggested fix
- background-color: currentColor; + background-color: currentcolor;🤖 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/theme/default.less` at line 288, The CSS rule setting the sorter icon color uses the keyword with incorrect casing; update the background-color value from "currentColor" to the lint-expected lowercase "currentcolor" in the rule that sets the sorter icon color (i.e., the background-color property for the sorter icon in default.less) so it satisfies the value-keyword-case rule.
113-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid hiding sorter/filter controls with
visibility: hiddenin header cells.Line 116 and Line 124/132 keep controls visibility-toggled and only reveal on hover, which risks keyboard-access regression for sorter/filter affordances. Keep opacity-based hiding and reveal on both hover and focus-within.
Suggested fix
.ant-table-column-sorter, .ant-table-filter-trigger { opacity: 0; - visibility: hidden; transition: opacity 0.2s ease, color 0.2s ease, background-color 0.2s ease; } - &:hover { + &:hover, + &:focus-within { .ant-table-column-sorter, .ant-table-filter-trigger { opacity: 1; - visibility: visible; } } .ant-table-column-sorter:has(.ant-table-column-sorter-up.active), .ant-table-column-sorter:has(.ant-table-column-sorter-down.active), .ant-table-filter-trigger.active { opacity: 1; - visibility: visible; }🤖 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/theme/default.less` around lines 113 - 133, The current rules hide .ant-table-column-sorter and .ant-table-filter-trigger using visibility: hidden which prevents keyboard focus; instead keep opacity transitions but remove visibility toggling and add focus rules so controls are revealed on both :hover and :focus-within (and when the sorter/filter has .active). Update the selectors shown (e.g., .ant-table-column-sorter, .ant-table-filter-trigger, the &:hover block, and the rules targeting :has(... .active) and .ant-table-filter-trigger.active) to not set visibility: hidden and to include :focus-within alongside :hover so keyboard users can access sorter/filter controls.
🧹 Nitpick comments (2)
src/pages/alertCurEvent/style.less (2)
166-170: ⚡ Quick winReduce selector nesting depth for tag content overflow rule.
The nested
> divunder.alert-event-tags .ant-tagexceeds the Less nesting-depth limit; prefer a dedicated class on the inner content element and target that class directly.As per coding guidelines, "Keep selectors locally scoped; limit nesting depth to ≤3 levels in Less/CSS files".
🤖 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/alertCurEvent/style.less` around lines 166 - 170, The Less nesting for ".alert-event-tags .ant-tag > div" is too deep; create a dedicated class on the inner element (e.g., add a class like "tag-content" to the div inside the .ant-tag) and replace the nested selector with a direct rule targeting that class (e.g., ".alert-event-tags .ant-tag .tag-content" or simply ".tag-content") that applies overflow: hidden; white-space: nowrap; text-overflow: ellipsis; so the selector depth stays ≤3; update the template/JSX to add the new class to the inner div referenced by .ant-tag.
111-134: ⚡ Quick winMove toolbar/button layout styling to Tailwind utilities in TSX.
These new rules are direct layout/spacing/sizing styles and are better kept inline with Tailwind utility classes to avoid extra Less indirection.
As per coding guidelines, "Use Tailwind utility classes for container layout (flex, grid, gap, padding, margin, alignment)" and "Use Tailwind CSS for direct expressible styles (layout, spacing, typography, border-radius, common interaction states); avoid Tailwind for cases where it's not practical or impossible".
🤖 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/alertCurEvent/style.less` around lines 111 - 134, The Less rules for .alert-event-summary-toolbar, its first child, .alert-event-expand-btn, and .ant-btn-icon should be removed and replaced by Tailwind utility classes in the TSX: give the toolbar container flex items-center justify-end gap-2, apply flex-1 min-w-0 to the left/first child element, set the expand button to flex-none w-7 h-7 p-0, and add inline-flex items-center justify-center to the icon element (ant-btn-icon) in the button; update the JSX className props accordingly and delete the matching Less selectors.
🤖 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/alertCurEvent/pages/List/index.tsx`:
- Around line 330-339: The icon-only toggle Button (className
'alert-event-expand-btn') lacks accessible semantics; update the Button in this
component to include an explicit accessible name and state by adding aria-label
based on eventColumnExpanded (use the same t('common:btn.expand') /
t('common:btn.collapse') strings) and an appropriate toggle state attribute
(aria-pressed or aria-expanded) tied to eventColumnExpanded, leaving the
existing onClick handler (setEventColumnExpanded) and Tooltip intact so
assistive tech can announce the button and its current state.
In `@src/theme/default.less`:
- Line 265: Replace the hardcoded "`#000`" in the color-mix call with the proper
theme token instead of a magic value; update the rule using color-mix(in srgb,
var(--fc-fill-error) 82%, var(--fc-fill-foreground)) (or the equivalent
foreground/neutral token from src/theme/variable.css used across themes) so the
danger hover color uses theme variables and respects light/dark variants.
---
Outside diff comments:
In `@src/pages/alertCurEvent/pages/List/AlertTable.tsx`:
- Around line 305-306: The table pagination defaults are incorrect: in
AlertTable update the pagination config so defaultPageSize is 15 (not 30) and
ensure the pageSizeOptions array includes 15 (e.g., [..., 15, ...]) so the
default matches the selectable options; locate the pagination object where
defaultPageSize and pageSizeOptions are defined and change the values there
(references: defaultPageSize, pageSizeOptions in AlertTable component).
---
Duplicate comments:
In `@src/pages/alertCurEvent/pages/List/AlertTable.tsx`:
- Line 195: The operations column renderer in AlertTable.tsx is using the wrong
parameter signature (render(record)) so it receives the cell value instead of
the row object; update the render functions for the operations column(s) to
accept the value placeholder and the row object (e.g., render(_, record))
wherever used (notably the render at the current function around the operations
column and the other render occurrences you flagged between lines ~206-226 and
~241-244) and then adjust any references that expect the full row (e.g.,
accesses to record.id, record.status, etc.) to use the second parameter
(record).
In `@src/theme/default.less`:
- Line 288: The CSS rule setting the sorter icon color uses the keyword with
incorrect casing; update the background-color value from "currentColor" to the
lint-expected lowercase "currentcolor" in the rule that sets the sorter icon
color (i.e., the background-color property for the sorter icon in default.less)
so it satisfies the value-keyword-case rule.
- Around line 113-133: The current rules hide .ant-table-column-sorter and
.ant-table-filter-trigger using visibility: hidden which prevents keyboard
focus; instead keep opacity transitions but remove visibility toggling and add
focus rules so controls are revealed on both :hover and :focus-within (and when
the sorter/filter has .active). Update the selectors shown (e.g.,
.ant-table-column-sorter, .ant-table-filter-trigger, the &:hover block, and the
rules targeting :has(... .active) and .ant-table-filter-trigger.active) to not
set visibility: hidden and to include :focus-within alongside :hover so keyboard
users can access sorter/filter controls.
---
Nitpick comments:
In `@src/pages/alertCurEvent/style.less`:
- Around line 166-170: The Less nesting for ".alert-event-tags .ant-tag > div"
is too deep; create a dedicated class on the inner element (e.g., add a class
like "tag-content" to the div inside the .ant-tag) and replace the nested
selector with a direct rule targeting that class (e.g., ".alert-event-tags
.ant-tag .tag-content" or simply ".tag-content") that applies overflow: hidden;
white-space: nowrap; text-overflow: ellipsis; so the selector depth stays ≤3;
update the template/JSX to add the new class to the inner div referenced by
.ant-tag.
- Around line 111-134: The Less rules for .alert-event-summary-toolbar, its
first child, .alert-event-expand-btn, and .ant-btn-icon should be removed and
replaced by Tailwind utility classes in the TSX: give the toolbar container flex
items-center justify-end gap-2, apply flex-1 min-w-0 to the left/first child
element, set the expand button to flex-none w-7 h-7 p-0, and add inline-flex
items-center justify-center to the icon element (ant-btn-icon) in the button;
update the JSX className props accordingly and delete the matching Less
selectors.
🪄 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: 3c5a399f-0392-4c94-8f2e-d30100cccd65
📒 Files selected for processing (10)
scripts/generate_antd_dark_less.jsscripts/generate_antd_gold_less.jssrc/pages/alertCurEvent/pages/List/AlertTable.tsxsrc/pages/alertCurEvent/pages/List/index.tsxsrc/pages/alertCurEvent/style.lesssrc/theme/antd.dark.lesssrc/theme/antd.light-blue.lesssrc/theme/antd.light-gold.lesssrc/theme/default.lessvite.config.ts
✅ Files skipped from review due to trivial changes (1)
- src/theme/antd.dark.less
🚧 Files skipped from review as they are similar to previous changes (1)
- vite.config.ts
Comment on lines
+330
to
+339
| <Tooltip title={eventColumnExpanded ? t('common:btn.collapse') : t('common:btn.expand')}> | ||
| <Button | ||
| type='text' | ||
| size='small' | ||
| className='alert-event-expand-btn' | ||
| icon={eventColumnExpanded ? <ListChevronsDownUp size={14} /> : <ListChevronsUpDown size={14} />} | ||
| onClick={() => { | ||
| setEventColumnExpanded(!eventColumnExpanded); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Add accessible semantics to the icon-only toggle button.
The expand/collapse control is icon-only, so it needs an explicit accessible name and pressed state for screen readers/assistive tech.
Proposed fix
<Button
type='text'
size='small'
className='alert-event-expand-btn'
+ aria-label={eventColumnExpanded ? t('common:btn.collapse') : t('common:btn.expand')}
+ aria-pressed={eventColumnExpanded}
icon={eventColumnExpanded ? <ListChevronsDownUp size={14} /> : <ListChevronsUpDown size={14} />}
onClick={() => {
- setEventColumnExpanded(!eventColumnExpanded);
+ setEventColumnExpanded((prev) => !prev);
}}
/>🤖 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/alertCurEvent/pages/List/index.tsx` around lines 330 - 339, The
icon-only toggle Button (className 'alert-event-expand-btn') lacks accessible
semantics; update the Button in this component to include an explicit accessible
name and state by adding aria-label based on eventColumnExpanded (use the same
t('common:btn.expand') / t('common:btn.collapse') strings) and an appropriate
toggle state attribute (aria-pressed or aria-expanded) tied to
eventColumnExpanded, leaving the existing onClick handler
(setEventColumnExpanded) and Tooltip intact so assistive tech can announce the
button and its current state.
|
|
||
| .fc-table-action-menu-button.ant-btn-dangerous:hover, | ||
| .fc-table-action-menu-button.ant-btn-dangerous:focus { | ||
| color: color-mix(in srgb, var(--fc-fill-error) 82%, #000); |
There was a problem hiding this comment.
Replace hardcoded black in danger hover color mix with a theme token.
Line 265 introduces #000, which breaks theme-token consistency and can drift across light/dark variants.
As per coding guidelines, "Use color and theme-related values from src/theme/variable.css and existing theme system; avoid magic color values."
🤖 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/theme/default.less` at line 265, Replace the hardcoded "`#000`" in the
color-mix call with the proper theme token instead of a magic value; update the
rule using color-mix(in srgb, var(--fc-fill-error) 82%,
var(--fc-fill-foreground)) (or the equivalent foreground/neutral token from
src/theme/variable.css used across themes) so the danger hover color uses theme
variables and respects light/dark variants.
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 (4)
src/pages/builtInComponents/Dashboards/index.tsx (1)
267-271:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd failure handling for payload deletion.
The delete confirm flow only handles success; request failures are currently silent.
As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include 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/builtInComponents/Dashboards/index.tsx` around lines 267 - 271, The deletion confirm handler currently calls deletePayloads([record.id]).then(() => fetchData()) without handling failures; update the onOk handler in the component (the function named onOk) to await or attach a .catch to deletePayloads([record.id]) and handle errors (e.g., show an error notification/toast and log the error) while still refreshing via fetchData() only on success; follow existing project patterns for error handling (try/catch around the await or .catch with processLogger/notification) so failures are not silent.src/pages/warning/subscribe/ListNG.tsx (1)
308-315:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix delete error feedback path (wrong toast type + missing rejection handling).
res.errcurrently triggersmessage.success(...), and rejected delete requests are not handled.Suggested minimal patch
onOk: () => { - deleteSubscribes({ ids: [record.id] }, record.group_id).then((res) => { - refreshList(); - if (res.err) { - message.success(res.err); - } else { - message.success(t('common:success.delete')); - } - }); + deleteSubscribes({ ids: [record.id] }, record.group_id) + .then((res) => { + if (res.err) { + message.error(res.err); + return; + } + refreshList(); + message.success(t('common:success.delete')); + }) + .catch(() => { + message.error(t('common:error.delete')); + }); },As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include 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/warning/subscribe/ListNG.tsx` around lines 308 - 315, The delete callback misreports errors and doesn't handle promise rejections: change the flow around deleteSubscribes({ ids: [record.id] }, record.group_id) so that errors use message.error(...) (not message.success), move refreshList() to run only after a successful delete, and add proper rejection handling (either await inside try/catch or append .catch) to surface network/exception failures (use message.error(t('common:failed.delete') or the returned res.err) and optionally log the error). Ensure you update the handler around deleteSubscribes, the success/error branches that call message, and include a .catch or try/catch to handle thrown errors.src/pages/dashboard/List/index.tsx (1)
344-366:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap action-menu async operations with explicit error handling.
Clone/delete/export handlers await async calls without a
try/catch, so failures won’t provide consistent user feedback.As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include error handling (try/catch,.catch,onError, etc.) consistent with existing patterns.Also applies to: 381-385
🤖 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/dashboard/List/index.tsx` around lines 344 - 366, The async menu actions (cloneDashboard, delete handler, and getDashboard/Export) lack error handling; wrap each async call in a try/catch so failures show user feedback and avoid side-effects on error. For the clone flow (cloneDashboard) and the export flow (getDashboard + Export + exportDataStringify) update the onClick/onOk handlers to await inside try blocks, call message.success on success and message.error with the caught Error.message (and optionally console.error) on failure, and only call setRefreshKey or other success-only effects after the awaited call succeeds; apply the same pattern to the delete handler referenced around lines 381-385.src/pages/alertRules/List/ListNG.tsx (1)
309-313:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle delete failures in the operations dropdown.
deleteStrategy(...).then(...)has no rejection handling, so failed deletions are silent.As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include 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/alertRules/List/ListNG.tsx` around lines 309 - 313, The onOk handler for the delete confirmation uses deleteStrategy([record.id], record.group_id).then(...) without handling rejections; update the onOk callback in ListNG.tsx (the function that calls deleteStrategy) to handle errors by either using async/await with try/catch or appending .catch(...) to the promise, and on failure call message.error(...) (and optionally log the error) so fetchData() is only called on success and failures surface to the user/developer instead of being silent.
♻️ Duplicate comments (4)
src/pages/notificationChannels/pages/ListNG/index.tsx (1)
323-327:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd rejection handling to the delete confirmation flow.
The delete action still only handles success; failures need explicit error feedback.
As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include 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/notificationChannels/pages/ListNG/index.tsx` around lines 323 - 327, The onOk handler for the delete confirmation calls deleteItems([record.id]) but lacks rejection handling; update the onOk logic (the callback passed to the confirmation dialog) to handle failures by catching promise rejections from deleteItems (either with .catch(...) or by making the handler async and using try/catch), and on error call message.error(...) with a useful message (include the translated string via t(...) and/or the error.message) while preserving the success path that calls message.success(...) and run(); reference the onOk callback, deleteItems, message.success, message.error, t and run to locate and update the code.src/pages/recordingRules/PageTable.tsx (1)
243-247:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle delete request failures in row actions.
This delete path still only handles success; failures should surface an error message and avoid silent no-op behavior.
As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include 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/recordingRules/PageTable.tsx` around lines 243 - 247, The onOk handler for the row delete confirmation currently calls deleteRecordingRule([record.id], record.group_id) and only handles the success case; update the onOk callback in PageTable to handle failures by attaching a .catch (or using async/await with try/catch) to deleteRecordingRule and calling message.error(...) with the error message, ensuring refreshList() is only invoked on success and not on failure. Specifically, modify the onOk closure that calls deleteRecordingRule to capture errors from deleteRecordingRule and surface them via message.error, mirroring existing async error-handling patterns used elsewhere in this file.src/pages/embeddedProduct/pages/List/index.tsx (1)
156-162:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle delete failures in the confirmation flow.
onOkonly handles success; failed deletes currently have no user-visible feedback and no explicit rejection handling.Suggested minimal patch
onOk: () => { - return deleteEmbeddedProducts(String(record.id)).then(() => { - message.success(t('common:success.delete')); - fetchData(); - eventBus.emit(EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED); - }); + return deleteEmbeddedProducts(String(record.id)) + .then(() => { + message.success(t('common:success.delete')); + fetchData(); + eventBus.emit(EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED); + }) + .catch(() => { + message.error(t('common:error.delete')); + }); },As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include 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/embeddedProduct/pages/List/index.tsx` around lines 156 - 162, The confirmation onOk handler for deleting embedded products currently only handles the success path; update the onOk callback that calls deleteEmbeddedProducts(String(record.id)) to handle failures by adding proper error handling (e.g., chain a .catch or use try/catch if converted to async) so that on error you call message.error(...) with a meaningful message, ensure fetchData() and eventBus.emit(EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED) only run on success, and propagate or return the rejected promise so the caller knows the operation failed; reference the existing onOk, deleteEmbeddedProducts, message.success, message.error, fetchData, eventBus.emit, and EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED symbols when making the change.src/pages/alertRules/List/ListNG.tsx (1)
274-275:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix operations column render signature to receive row data correctly.
render: (record: any) =>is using the first antd render arg as row data. For this column, row data must be the second arg ((_, record)), otherwiserecord.id/record.cateaccesses are invalid at runtime.#!/bin/bash # Verify operations column render signature and surrounding column shape. rg -n -C3 "title:\s*t\('common:table.operations'\)|render:\s*\(record:\s*any\)\s*=>" src/pages/alertRules/List/ListNG.tsx🤖 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/List/ListNG.tsx` around lines 274 - 275, The operations column's render callback currently uses the first antd render arg as row data (render: (record: any) => ...), causing invalid accesses like record.id and record.cate; update the render signature to accept the two-arg form (render: (_: any, record: any) => ...) and then use the second parameter (record) within the body; locate the column definition with title t('common:table.operations') and change its render to (_ , record: any) => { ... } so row data is read correctly from record for IDs/cate accesses.
🤖 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/alertRules/List/ListNG.tsx`:
- Around line 309-313: The onOk handler for the delete confirmation uses
deleteStrategy([record.id], record.group_id).then(...) without handling
rejections; update the onOk callback in ListNG.tsx (the function that calls
deleteStrategy) to handle errors by either using async/await with try/catch or
appending .catch(...) to the promise, and on failure call message.error(...)
(and optionally log the error) so fetchData() is only called on success and
failures surface to the user/developer instead of being silent.
In `@src/pages/builtInComponents/Dashboards/index.tsx`:
- Around line 267-271: The deletion confirm handler currently calls
deletePayloads([record.id]).then(() => fetchData()) without handling failures;
update the onOk handler in the component (the function named onOk) to await or
attach a .catch to deletePayloads([record.id]) and handle errors (e.g., show an
error notification/toast and log the error) while still refreshing via
fetchData() only on success; follow existing project patterns for error handling
(try/catch around the await or .catch with processLogger/notification) so
failures are not silent.
In `@src/pages/dashboard/List/index.tsx`:
- Around line 344-366: The async menu actions (cloneDashboard, delete handler,
and getDashboard/Export) lack error handling; wrap each async call in a
try/catch so failures show user feedback and avoid side-effects on error. For
the clone flow (cloneDashboard) and the export flow (getDashboard + Export +
exportDataStringify) update the onClick/onOk handlers to await inside try
blocks, call message.success on success and message.error with the caught
Error.message (and optionally console.error) on failure, and only call
setRefreshKey or other success-only effects after the awaited call succeeds;
apply the same pattern to the delete handler referenced around lines 381-385.
In `@src/pages/warning/subscribe/ListNG.tsx`:
- Around line 308-315: The delete callback misreports errors and doesn't handle
promise rejections: change the flow around deleteSubscribes({ ids: [record.id]
}, record.group_id) so that errors use message.error(...) (not message.success),
move refreshList() to run only after a successful delete, and add proper
rejection handling (either await inside try/catch or append .catch) to surface
network/exception failures (use message.error(t('common:failed.delete') or the
returned res.err) and optionally log the error). Ensure you update the handler
around deleteSubscribes, the success/error branches that call message, and
include a .catch or try/catch to handle thrown errors.
---
Duplicate comments:
In `@src/pages/alertRules/List/ListNG.tsx`:
- Around line 274-275: The operations column's render callback currently uses
the first antd render arg as row data (render: (record: any) => ...), causing
invalid accesses like record.id and record.cate; update the render signature to
accept the two-arg form (render: (_: any, record: any) => ...) and then use the
second parameter (record) within the body; locate the column definition with
title t('common:table.operations') and change its render to (_ , record: any) =>
{ ... } so row data is read correctly from record for IDs/cate accesses.
In `@src/pages/embeddedProduct/pages/List/index.tsx`:
- Around line 156-162: The confirmation onOk handler for deleting embedded
products currently only handles the success path; update the onOk callback that
calls deleteEmbeddedProducts(String(record.id)) to handle failures by adding
proper error handling (e.g., chain a .catch or use try/catch if converted to
async) so that on error you call message.error(...) with a meaningful message,
ensure fetchData() and eventBus.emit(EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED) only
run on success, and propagate or return the rejected promise so the caller knows
the operation failed; reference the existing onOk, deleteEmbeddedProducts,
message.success, message.error, fetchData, eventBus.emit, and
EVENT_KEYS.EMBEDDED_PRODUCT_UPDATED symbols when making the change.
In `@src/pages/notificationChannels/pages/ListNG/index.tsx`:
- Around line 323-327: The onOk handler for the delete confirmation calls
deleteItems([record.id]) but lacks rejection handling; update the onOk logic
(the callback passed to the confirmation dialog) to handle failures by catching
promise rejections from deleteItems (either with .catch(...) or by making the
handler async and using try/catch), and on error call message.error(...) with a
useful message (include the translated string via t(...) and/or the
error.message) while preserving the success path that calls message.success(...)
and run(); reference the onOk callback, deleteItems, message.success,
message.error, t and run to locate and update the code.
In `@src/pages/recordingRules/PageTable.tsx`:
- Around line 243-247: The onOk handler for the row delete confirmation
currently calls deleteRecordingRule([record.id], record.group_id) and only
handles the success case; update the onOk callback in PageTable to handle
failures by attaching a .catch (or using async/await with try/catch) to
deleteRecordingRule and calling message.error(...) with the error message,
ensuring refreshList() is only invoked on success and not on failure.
Specifically, modify the onOk closure that calls deleteRecordingRule to capture
errors from deleteRecordingRule and surface them via message.error, mirroring
existing async error-handling patterns used elsewhere in this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c8695fe-89ed-4b68-8879-f188540211e5
📒 Files selected for processing (7)
src/pages/alertRules/List/ListNG.tsxsrc/pages/builtInComponents/Dashboards/index.tsxsrc/pages/dashboard/List/index.tsxsrc/pages/embeddedProduct/pages/List/index.tsxsrc/pages/notificationChannels/pages/ListNG/index.tsxsrc/pages/recordingRules/PageTable.tsxsrc/pages/warning/subscribe/ListNG.tsx
Standardize all table action column Dropdowns to use `click`-only trigger and explicit `align` prop for consistent positioning. Includes minor formatting fixes (indentation, JSX simplification).
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 (13)
src/pages/aiConfig/llmConfigs/pages/List.tsx (1)
118-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd error handling for LLM config delete action.
The confirm
onOkpath handles only success; add rejection handling and user feedback.As per coding guidelines, "Async requests must include 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/aiConfig/llmConfigs/pages/List.tsx` around lines 118 - 123, The confirm onOk handler for deleting an LLM config currently calls deleteItem(record.id).then(...) without handling failures; update the onOk to handle rejections (either convert the handler to async/await with try/catch around await deleteItem(record.id) or append .catch(...) to the promise) and show feedback on error (e.g., message.error with the error message) while preserving the success path that calls message.success and run(); ensure you reference the existing deleteItem, onOk handler, message.success, run, and record.id symbols when making the change.src/pages/builtInComponents/CollectTpls/index.tsx (1)
218-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle delete failures in collect template actions.
deletePayloadsinside confirm only executes success flow. Add.catchto surface errors and avoid silent failures.As per coding guidelines, "Async requests must include 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/builtInComponents/CollectTpls/index.tsx` around lines 218 - 223, The confirm handler calls deletePayloads([record.id]) but only handles the success path; add error handling so failures are surfaced and the success callbacks (fetchData, fetchCates) only run on success. Update the onOk handler for the confirm action (the function that calls deletePayloads) to either await deletePayloads in a try/catch or append .catch(...) to the promise; in the catch block log or show the error (e.g., using message.error or processLogger/error handler) and avoid calling fetchData()/fetchCates() when deletion fails so failures are not silently ignored.src/pages/aiConfig/mcpServers/pages/List.tsx (1)
103-108:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd failure handling for MCP server delete confirmation.
deleteItem(record.id)currently has no rejection branch inonOk.As per coding guidelines, "Async requests must include 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/aiConfig/mcpServers/pages/List.tsx` around lines 103 - 108, The delete confirmation's onOk handler calls deleteItem(record.id) but lacks error handling; update the onOk callback in List.tsx to handle rejection from deleteItem (e.g., attach a .catch or use async/try-catch) so failures show a user-facing message and avoid silent failures — call message.error with a localized error string (using t) and optionally log the error, while keeping the success path (message.success and run()) intact; reference the onOk handler, deleteItem, run, and message.success/message.error to find where to add the catch/try-catch.src/pages/metricsBuiltin/List.tsx (1)
320-327:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd error handling to row delete confirm flow.
Line 323 only handles success; failed deletes are silent and don’t surface feedback.
Proposed fix
Modal.confirm({ title: t('common:confirm.delete'), - onOk() { - deleteMetrics([record.id]).then(() => { - message.success(t('common:success.delete')); - setRefreshFlag(_.uniqueId('refreshFlag_')); - }); + onOk: async () => { + try { + await deleteMetrics([record.id]); + message.success(t('common:success.delete')); + setRefreshFlag(_.uniqueId('refreshFlag_')); + } catch (err: any) { + message.error(err?.message || t('common:failure.delete')); + return Promise.reject(err); + } }, });As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include 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/metricsBuiltin/List.tsx` around lines 320 - 327, The Modal.confirm onOk currently calls deleteMetrics([record.id]) and only handles success; update the onOk handler (Modal.confirm's onOk) to handle errors too by either making onOk async with try/catch or attaching a .catch to deleteMetrics([record.id]); on success call message.success and setRefreshFlag(_.uniqueId('refreshFlag_')), on failure call message.error with a user-friendly localized message (e.g., t('common:failed.delete')) and/or include the error details for logging—ensure deleteMetrics, message.success, message.error, setRefreshFlag, _.uniqueId and record.id are used appropriately and that refresh is only triggered on success.src/pages/aiConfig/agents/pages/List.tsx (1)
88-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle failures for enable toggle and delete actions.
Both mutation paths currently assume success; failed requests won’t show feedback and may desync UI state.
Proposed fix
onChange={(checked) => { - putItem(record.id, { - ...record, - enabled: checked, - }).then(() => { - message.success(t('common:success.modify')); - run(); - }); + putItem(record.id, { + ...record, + enabled: checked, + }) + .then(() => { + message.success(t('common:success.modify')); + run(); + }) + .catch((err) => { + message.error(err?.message || t('common:failure.modify')); + }); }} @@ onOk: () => { - deleteItem(record.id).then(() => { - message.success(t('common:success.delete')); - run(); - }); + deleteItem(record.id) + .then(() => { + message.success(t('common:success.delete')); + run(); + }) + .catch((err) => { + message.error(err?.message || t('common:failure.delete')); + }); },As per coding guidelines,
src/**/*.{ts,tsx}: Async requests must include error handling (try/catch,.catch,onError, etc.) consistent with existing patterns.Also applies to: 135-140
🤖 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/agents/pages/List.tsx` around lines 88 - 95, The enable-toggle and delete mutation calls (the putItem(...) promise and the delete path around lines referencing putItem, run, and message.success) lack error handling and assume success; wrap these async operations with proper error handling (use .catch or async/await with try/catch) and on error call message.error with a helpful localized message, avoid leaving the UI in the optimistic state by reverting any local changes or re-running run() only on success, and ensure the delete handler does the same (handle errors for the delete function referenced near lines 135-140) so failures surface to the user and the UI stays consistent.src/pages/builtInComponents/AlertRules/index.tsx (1)
312-317:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDelete action should handle request failures.
deletePayloads([record.id]).then(...)is success-only; add a rejection path and keep refresh calls in the success path.As per coding guidelines, "Async requests must include error handling (`try/catch`, `.catch`, `onError`, etc.) consistent with existing patterns".Suggested patch
onOk() { - deletePayloads([record.id]).then(() => { - fetchData(); - fetchCates(); - }); + return deletePayloads([record.id]) + .then(() => { + fetchData(); + fetchCates(); + message.success(t('common:success.delete')); + }) + .catch((err) => { + message.error(err?.message || t('common:error.delete')); + return Promise.reject(err); + }); },🤖 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/builtInComponents/AlertRules/index.tsx` around lines 312 - 317, The delete handler currently calls deletePayloads([record.id]).then(...) without handling failures; update the onOk() implementation for the delete action so that the refresh calls (fetchData and fetchCates) only run on successful deletion and errors are handled—either by appending a .catch(...) to deletePayloads(...) to log or surface the error (and avoid calling fetchData/fetchCates), or by using async/await with try/catch inside onOk to call deletePayloads and then on success call fetchData() and fetchCates(), and in the catch block handle/report the error consistently with existing patterns.src/pages/aiConfig/skills/pages/ResourcesTable.tsx (1)
177-182:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle delete failures in the row action flow.
Line 178 only handles success. Add a rejection path so users get feedback and
run()is not called on failure.As per coding guidelines, "Async requests must include error handling (`try/catch`, `.catch`, `onError`, etc.) consistent with existing patterns".Suggested patch
onOk: () => { - deleteFile(record.id).then(() => { - message.success(t('common:success.delete')); - run(); - }); + return deleteFile(record.id) + .then(() => { + message.success(t('common:success.delete')); + run(); + }) + .catch((err) => { + message.error(err?.message || t('common:error.delete')); + return Promise.reject(err); + }); },🤖 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/ResourcesTable.tsx` around lines 177 - 182, The delete confirmation's onOk currently calls deleteFile(record.id) and only handles the success path; update it to handle failures by adding a rejection handler (e.g., .catch or try/catch if converted to async) so that on error you call message.error(...) with the error or a translated message and do not call run(); ensure the onOk flow references deleteFile(record.id) and run() so the success path still shows message.success(...) and calls run(), while any rejection displays the error feedback and prevents run() from executing.src/pages/user/users.tsx (1)
167-170:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUser delete action needs rejection handling.
The current
deleteUser(record.id).then(...)flow has no error branch, so delete failures are silent.As per coding guidelines, "Async requests must include error handling (`try/catch`, `.catch`, `onError`, etc.) consistent with existing patterns".Suggested patch
onOk: () => { - deleteUser(record.id).then((_) => { - message.success(t('common:success.delete')); - handleClose(); - }); + return deleteUser(record.id) + .then(() => { + message.success(t('common:success.delete')); + handleClose(); + }) + .catch((err) => { + message.error(err?.message || t('common:error.delete')); + return Promise.reject(err); + }); },🤖 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/user/users.tsx` around lines 167 - 170, The deleteUser call currently only handles the success path and swallows failures; update the delete flow in users.tsx (the deleteUser(record.id) invocation) to handle rejections by adding an error branch (either append .catch(...) to the promise or convert the handler to async/await with try/catch) that calls message.error(...) with a useful localized error message and logs the error, and ensure handleClose() is only called on success or that failures leave the UI in a consistent state.src/pages/builtInComponents/Dashboards/index.tsx (2)
267-271:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle delete failures in modal
onOk.
deletePayloads(...).then(...)only handles success and misses user feedback on failure.Proposed fix
- onOk() { - deletePayloads([record.id]).then(() => { - fetchData(); - }); - }, + async onOk() { + try { + await deletePayloads([record.id]); + fetchData(); + } catch (err) { + message.error(t('common:failed.delete')); + return Promise.reject(err); + } + },As per coding guidelines, "Async requests must include 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/builtInComponents/Dashboards/index.tsx` around lines 267 - 271, The modal onOk handler currently calls deletePayloads([record.id]).then(() => fetchData()) with no error handling; update the onOk (or the function containing it) to await or attach a .catch to deletePayloads([record.id]) and handle failures by showing user feedback (e.g., message.error or notification.error) and ensuring any UI state (modal loading/visibility) is updated appropriately, while preserving the success path that calls fetchData() and shows success feedback; reference the onOk handler, deletePayloads, and fetchData to locate and modify the code.
200-214:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix AntD Table render callback signature in operations column.
The render function receives three parameters
(value, record, index), but the current code only accepts one and names itrecord. This captures the cell value (first parameter) instead of the row record object, causingrecord.content,record.updated_by, andrecord.idto be undefined at runtime.Proposed fix
- render: (record) => { + render: (_, record) => { return (🤖 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/builtInComponents/Dashboards/index.tsx` around lines 200 - 214, The Table column's render callback currently declares a single parameter named record but actually receives (value, record, index), so the code is reading the cell value instead of the row object; update the render signature to accept the standard parameters (value, record, index) or use (_, record) if you only need the row, then update usages in this callback (references to record.content, record.updated_by, record.id used by Import, formatBeautifyJson, TableActionButton handlers, etc.) to use the correct record parameter so those fields are defined at runtime.src/pages/alertRules/List/ListNG.tsx (1)
309-313:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle delete request failures in
Modal.confirmaction.
deleteStrategy(...).then(...)has no failure path, so errors are silent and users get no feedback.Proposed fix
- onOk: () => { - deleteStrategy([record.id], record.group_id).then(() => { - message.success(t('common:success.delete')); - fetchData(); - }); - }, + onOk: async () => { + try { + await deleteStrategy([record.id], record.group_id); + message.success(t('common:success.delete')); + fetchData(); + } catch (err) { + message.error(t('common:failed.delete')); + return Promise.reject(err); + } + },As per coding guidelines, "Async requests must include 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/alertRules/List/ListNG.tsx` around lines 309 - 313, The Modal.confirm onOk currently calls deleteStrategy([record.id], record.group_id).then(...) with no failure handling, so failures are silent; update the onOk handler (the Modal.confirm callback that calls deleteStrategy) to handle errors either by using async/await with try/catch or by adding a .catch handler to the promise returned by deleteStrategy, and on error call message.error with a descriptive message (and optionally log the error) while preserving the success branch that calls message.success and fetchData; ensure you reference the same identifiers (deleteStrategy, record.id, record.group_id, message.success, message.error, fetchData) so error feedback is shown to users.src/pages/dashboard/List/index.tsx (1)
344-365:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit failure handling for operations menu async actions.
Clone/export/delete calls run without
try/catch, so users may get no actionable feedback on failure.As per coding guidelines, "Async requests must include error handling (
try/catch,.catch,onError, etc.) consistent with existing patterns".Also applies to: 381-385
🤖 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/dashboard/List/index.tsx` around lines 344 - 365, The menu action callbacks (the clone flow using cloneDashboard in the onOk callback and the export flow using getDashboard/Export) lack error handling; wrap these async operations in try/catch blocks (and any similar async handlers around lines ~381-385) so failures call message.error(...) with a helpful localized message and optional logging, and only call message.success(...) and setRefreshKey(...) on success; ensure the same pattern is applied to delete/export/clone handlers (functions referenced: cloneDashboard, getDashboard, Export, setRefreshKey, message.success) so users receive clear feedback on errors.src/pages/warning/subscribe/ListNG.tsx (1)
308-315:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix delete error path and add rejection handling.
res.errcurrently shows viamessage.success, and rejected requests are not caught.Proposed fix
- onOk: () => { - deleteSubscribes({ ids: [record.id] }, record.group_id).then((res) => { - refreshList(); - if (res.err) { - message.success(res.err); - } else { - message.success(t('common:success.delete')); - } - }); - }, + onOk: async () => { + try { + const res = await deleteSubscribes({ ids: [record.id] }, record.group_id); + if (res.err) { + message.error(res.err); + return Promise.reject(new Error(res.err)); + } + message.success(t('common:success.delete')); + refreshList(); + } catch (err) { + message.error(t('common:failed.delete')); + return Promise.reject(err); + } + },As per coding guidelines, "Async requests must include 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/warning/subscribe/ListNG.tsx` around lines 308 - 315, The delete handler incorrectly shows errors as success and lacks rejection handling; update the deleteSubscribes call so it treats res.err as an error (use message.error) and only calls refreshList on successful deletion, and add a .catch to handle promise rejections (displaying err.message or String(err) via message.error). Locate the block that calls deleteSubscribes({ ids: [record.id] }, record.group_id) and replace the callback with logic that checks if (res.err) -> message.error(...), else -> message.success(t('common:success.delete')) and refreshList(), plus append .catch(err => message.error(err.message || String(err))).
♻️ Duplicate comments (8)
src/pages/variableConfigs/index.tsx (1)
173-177:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd error handling to variable-config delete confirmation.
onOkcurrently handles only success; rejected deletes fail silently.Suggested fix
onOk: () => { - deleteVariableConfigs(record.id).then(() => { + return deleteVariableConfigs(record.id).then(() => { message.success(t('common:success.delete')); fetchData(); + }).catch((err) => { + message.error(err?.message || t('common:request_failed')); + return Promise.reject(err); }); },As per coding guidelines, "Async requests must include 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/variableConfigs/index.tsx` around lines 173 - 177, The delete confirmation's onOk currently calls deleteVariableConfigs(record.id) without handling rejections; update the onOk handler to catch errors from deleteVariableConfigs (either by making onOk async and using try/catch around await deleteVariableConfigs(record.id) or by appending .catch(...) to the promise), show an error message via message.error (using t for localization) on failure, and keep the existing success path (message.success and fetchData()) in the success branch; reference the onOk callback, deleteVariableConfigs, record.id, message.success, message.error, t, and fetchData when making the change.src/pages/user/business.tsx (1)
98-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
deleteBusinessTeamMemberfailure path in confirm callback.The current flow only handles success. Add catch/try-catch and show an error on failure.
As per coding guidelines, "Async requests must include 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/user/business.tsx` around lines 98 - 102, The confirm onOk callback currently calls deleteBusinessTeamMember(teamId, params) and only handles the success path; wrap the async call in a try/catch (or append .catch) so failures are handled: call message.error(...) with a clear failure message and optionally log the error, and ensure getTeamList() only runs on success; update the onOk handler around deleteBusinessTeamMember and getTeamList to reflect this error handling.src/pages/notificationRules/pages/List.tsx (2)
218-220:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign operations edit route with
IS_PLUSbehavior.This menu item always routes to
/${NS}/edit/${record.id}while the name link already branches for PLUS mode.Suggested fix
-<TableActionLink actionIcon='edit' to={{ pathname: `/${NS}/edit/${record.id}` }}> +<TableActionLink + actionIcon='edit' + to={{ pathname: IS_PLUS ? `/${NS}/detail/${record.id}` : `/${NS}/edit/${record.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/notificationRules/pages/List.tsx` around lines 218 - 220, The edit menu item always links to `/${NS}/edit/${record.id}` but must follow the same IS_PLUS branching as the name link; update the TableActionLink (in List.tsx) to build its `to`/pathname using the same conditional/utility used by the name link (check the name link's implementation and the IS_PLUS flag) so that when IS_PLUS is true it navigates to the PLUS-specific route and otherwise to the standard `edit` route, keeping the route variable references (NS, record.id) consistent.
235-238:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd error handling to notification-rule delete action.
The delete confirm
onOkonly implements success flow; rejected requests are unhandled.As per coding guidelines, "Async requests must include 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/notificationRules/pages/List.tsx` around lines 235 - 238, The onOk handler for the delete confirm currently calls deleteItems([record.id]).then(() => fetchData()) with no error handling; wrap the async call in a try/catch (or add a .catch) so rejections are handled: call deleteItems([record.id]) inside an async function or promise chain, on success call fetchData(), on error log the error and surface it to the user (e.g., message.error or notification.error) and clear any loading state. Update the onOk callback that references deleteItems and fetchData to follow the existing error-handling pattern used elsewhere in this file.src/pages/datasource/components/TableSource/index.tsx (1)
209-216:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle failed datasource mutations in the action menu.
Both status toggle and delete only implement success paths. Add rejection handling so failures are visible and refresh only occurs on success.
Suggested fix
onConfirm={() => { - updateDataSourceStatus({ + return updateDataSourceStatus({ id: record.id, status: record.status === 'enabled' ? 'disabled' : 'enabled', }).then(() => { message.success(record.status === 'enabled' ? t('success.disable') : t('success.enable')); setRefresh((oldVal) => !oldVal); + }).catch((err) => { + message.error(err?.message || t('common:request_failed')); + return Promise.reject(err); }); }} onOk() { - deleteDataSourceById(record.id).then(() => { + return deleteDataSourceById(record.id).then(() => { message.success(t('common:success.delete')); setRefresh((oldVal) => !oldVal); + }).catch((err) => { + message.error(err?.message || t('common:request_failed')); + return Promise.reject(err); }); },As per coding guidelines, "Async requests must include error handling (
try/catch,.catch,onError, etc.) consistent with existing patterns".Also applies to: 238-241
🤖 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/datasource/components/TableSource/index.tsx` around lines 209 - 216, The status-toggle and delete actions call updateDataSourceStatus and deleteDataSource but only handle the success path; wrap these async calls with proper rejection handling (use .catch or async/await try/catch) so failures show a visible error (call message.error with a descriptive message and the error) and ensure setRefresh((old) => !old) is only invoked on successful completion; target the updateDataSourceStatus and deleteDataSource call sites and ensure you use record.id/record.status for context in the messages.src/pages/alertRules/List/ListNG.tsx (1)
274-276:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix operations column
rendercallback signature to receive the row record.The callback currently treats the first argument as the row object, so
record.id/record.cateaccess can break at runtime.Proposed fix
- render: (record: any) => { + render: (_: unknown, record: AlertRuleType<any>) => { const anomalyEnabled = _.get(record, ['rule_config', 'anomaly_trigger', 'enable']);🤖 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/List/ListNG.tsx` around lines 274 - 276, The operations column's render callback currently declares a single parameter (render: (record: any) => { ... }) but Ant Design table passes (text, record, index), so the row object should be taken from the second parameter; change the signature to accept (text, record) or (value, record, index) and update all uses inside the callback (e.g., accesses like record.id, record.cate, _.get(record, [...])) to reference the second parameter `record` rather than the first positional arg; update the render declaration (the function used in the column definition) accordingly so runtime lookups for record.id and record.cate work correctly.src/pages/taskTpl/index.tsx (1)
196-203:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit error handling to template deletion
onOk.Current promise chain handles only success, so request failures are not handled deterministically.
Proposed fix
- onOk: () => { - return request(`${api.tasktpl(record.group_id)}/${record.id}`, { - method: 'DELETE', - }).then(() => { - message.success(t('msg.delete.success')); - refresh(); - }); - }, + onOk: async () => { + try { + await request(`${api.tasktpl(record.group_id)}/${record.id}`, { + method: 'DELETE', + }); + message.success(t('msg.delete.success')); + refresh(); + } catch (err) { + message.error(t('common:failed.delete')); + return Promise.reject(err); + } + },As per coding guidelines, "Async requests must include 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/taskTpl/index.tsx` around lines 196 - 203, The onOk handler for template deletion currently only handles the success path; wrap the request call in proper error handling so failures are handled deterministically: in the onOk of the delete confirm, call request(`${api.tasktpl(record.group_id)}/${record.id}`, { method: 'DELETE' }) and add a .catch (or use async/await with try/catch) to log or surface the error (e.g., message.error with the error message) and ensure refresh() is only called on success; reference the onOk handler, request function, api.tasktpl(...) and refresh() when locating where to add this handling.src/pages/recordingRules/PageTable.tsx (1)
244-247:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd failure handling for row delete action.
The delete flow still only handles success; failures are not surfaced.
Proposed fix
- onOk: () => { - deleteRecordingRule([record.id], record.group_id).then(() => { - message.success(t('common:success.delete')); - refreshList(); - }); - }, + onOk: async () => { + try { + await deleteRecordingRule([record.id], record.group_id); + message.success(t('common:success.delete')); + refreshList(); + } catch (err) { + message.error(t('common:failed.delete')); + return Promise.reject(err); + } + },As per coding guidelines, "Async requests must include 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/recordingRules/PageTable.tsx` around lines 244 - 247, The deleteRecordingRule call in the row delete handler currently only handles success and lacks failure handling; update the async flow around deleteRecordingRule([record.id], record.group_id) to catch errors (use .catch or async/try-catch) and on failure call message.error(...) with an appropriate translated error message and optionally log the error, while preserving the existing message.success(...) and refreshList() on success; ensure you reference the same identifiers (deleteRecordingRule, record.id, record.group_id, message.success, refreshList) so the behavior is consistent with other async request patterns in the codebase.
🤖 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/builtInComponents/Metrics/index.tsx`:
- Around line 199-203: The single-row delete call using
deleteMetrics([record.id]) currently only handles success; add error handling so
failures show feedback and do not refresh. Wrap the delete call in a proper
.catch (or convert to async/await with try/catch) around the promise returned by
deleteMetrics in the same block where message.success(...) and
setRefreshFlag(_.uniqueId('refreshFlag_')) are used; on error call
message.error(...) with an appropriate translated message (use t('common:...')
or include err.message) and avoid calling setRefreshFlag when the delete fails.
Ensure you reference deleteMetrics, message.success, message.error,
setRefreshFlag and _.uniqueId in the fix.
---
Outside diff comments:
In `@src/pages/aiConfig/agents/pages/List.tsx`:
- Around line 88-95: The enable-toggle and delete mutation calls (the
putItem(...) promise and the delete path around lines referencing putItem, run,
and message.success) lack error handling and assume success; wrap these async
operations with proper error handling (use .catch or async/await with try/catch)
and on error call message.error with a helpful localized message, avoid leaving
the UI in the optimistic state by reverting any local changes or re-running
run() only on success, and ensure the delete handler does the same (handle
errors for the delete function referenced near lines 135-140) so failures
surface to the user and the UI stays consistent.
In `@src/pages/aiConfig/llmConfigs/pages/List.tsx`:
- Around line 118-123: The confirm onOk handler for deleting an LLM config
currently calls deleteItem(record.id).then(...) without handling failures;
update the onOk to handle rejections (either convert the handler to async/await
with try/catch around await deleteItem(record.id) or append .catch(...) to the
promise) and show feedback on error (e.g., message.error with the error message)
while preserving the success path that calls message.success and run(); ensure
you reference the existing deleteItem, onOk handler, message.success, run, and
record.id symbols when making the change.
In `@src/pages/aiConfig/mcpServers/pages/List.tsx`:
- Around line 103-108: The delete confirmation's onOk handler calls
deleteItem(record.id) but lacks error handling; update the onOk callback in
List.tsx to handle rejection from deleteItem (e.g., attach a .catch or use
async/try-catch) so failures show a user-facing message and avoid silent
failures — call message.error with a localized error string (using t) and
optionally log the error, while keeping the success path (message.success and
run()) intact; reference the onOk handler, deleteItem, run, and
message.success/message.error to find where to add the catch/try-catch.
In `@src/pages/aiConfig/skills/pages/ResourcesTable.tsx`:
- Around line 177-182: The delete confirmation's onOk currently calls
deleteFile(record.id) and only handles the success path; update it to handle
failures by adding a rejection handler (e.g., .catch or try/catch if converted
to async) so that on error you call message.error(...) with the error or a
translated message and do not call run(); ensure the onOk flow references
deleteFile(record.id) and run() so the success path still shows
message.success(...) and calls run(), while any rejection displays the error
feedback and prevents run() from executing.
In `@src/pages/alertRules/List/ListNG.tsx`:
- Around line 309-313: The Modal.confirm onOk currently calls
deleteStrategy([record.id], record.group_id).then(...) with no failure handling,
so failures are silent; update the onOk handler (the Modal.confirm callback that
calls deleteStrategy) to handle errors either by using async/await with
try/catch or by adding a .catch handler to the promise returned by
deleteStrategy, and on error call message.error with a descriptive message (and
optionally log the error) while preserving the success branch that calls
message.success and fetchData; ensure you reference the same identifiers
(deleteStrategy, record.id, record.group_id, message.success, message.error,
fetchData) so error feedback is shown to users.
In `@src/pages/builtInComponents/AlertRules/index.tsx`:
- Around line 312-317: The delete handler currently calls
deletePayloads([record.id]).then(...) without handling failures; update the
onOk() implementation for the delete action so that the refresh calls (fetchData
and fetchCates) only run on successful deletion and errors are handled—either by
appending a .catch(...) to deletePayloads(...) to log or surface the error (and
avoid calling fetchData/fetchCates), or by using async/await with try/catch
inside onOk to call deletePayloads and then on success call fetchData() and
fetchCates(), and in the catch block handle/report the error consistently with
existing patterns.
In `@src/pages/builtInComponents/CollectTpls/index.tsx`:
- Around line 218-223: The confirm handler calls deletePayloads([record.id]) but
only handles the success path; add error handling so failures are surfaced and
the success callbacks (fetchData, fetchCates) only run on success. Update the
onOk handler for the confirm action (the function that calls deletePayloads) to
either await deletePayloads in a try/catch or append .catch(...) to the promise;
in the catch block log or show the error (e.g., using message.error or
processLogger/error handler) and avoid calling fetchData()/fetchCates() when
deletion fails so failures are not silently ignored.
In `@src/pages/builtInComponents/Dashboards/index.tsx`:
- Around line 267-271: The modal onOk handler currently calls
deletePayloads([record.id]).then(() => fetchData()) with no error handling;
update the onOk (or the function containing it) to await or attach a .catch to
deletePayloads([record.id]) and handle failures by showing user feedback (e.g.,
message.error or notification.error) and ensuring any UI state (modal
loading/visibility) is updated appropriately, while preserving the success path
that calls fetchData() and shows success feedback; reference the onOk handler,
deletePayloads, and fetchData to locate and modify the code.
- Around line 200-214: The Table column's render callback currently declares a
single parameter named record but actually receives (value, record, index), so
the code is reading the cell value instead of the row object; update the render
signature to accept the standard parameters (value, record, index) or use (_,
record) if you only need the row, then update usages in this callback
(references to record.content, record.updated_by, record.id used by Import,
formatBeautifyJson, TableActionButton handlers, etc.) to use the correct record
parameter so those fields are defined at runtime.
In `@src/pages/dashboard/List/index.tsx`:
- Around line 344-365: The menu action callbacks (the clone flow using
cloneDashboard in the onOk callback and the export flow using
getDashboard/Export) lack error handling; wrap these async operations in
try/catch blocks (and any similar async handlers around lines ~381-385) so
failures call message.error(...) with a helpful localized message and optional
logging, and only call message.success(...) and setRefreshKey(...) on success;
ensure the same pattern is applied to delete/export/clone handlers (functions
referenced: cloneDashboard, getDashboard, Export, setRefreshKey,
message.success) so users receive clear feedback on errors.
In `@src/pages/metricsBuiltin/List.tsx`:
- Around line 320-327: The Modal.confirm onOk currently calls
deleteMetrics([record.id]) and only handles success; update the onOk handler
(Modal.confirm's onOk) to handle errors too by either making onOk async with
try/catch or attaching a .catch to deleteMetrics([record.id]); on success call
message.success and setRefreshFlag(_.uniqueId('refreshFlag_')), on failure call
message.error with a user-friendly localized message (e.g.,
t('common:failed.delete')) and/or include the error details for logging—ensure
deleteMetrics, message.success, message.error, setRefreshFlag, _.uniqueId and
record.id are used appropriately and that refresh is only triggered on success.
In `@src/pages/user/users.tsx`:
- Around line 167-170: The deleteUser call currently only handles the success
path and swallows failures; update the delete flow in users.tsx (the
deleteUser(record.id) invocation) to handle rejections by adding an error branch
(either append .catch(...) to the promise or convert the handler to async/await
with try/catch) that calls message.error(...) with a useful localized error
message and logs the error, and ensure handleClose() is only called on success
or that failures leave the UI in a consistent state.
In `@src/pages/warning/subscribe/ListNG.tsx`:
- Around line 308-315: The delete handler incorrectly shows errors as success
and lacks rejection handling; update the deleteSubscribes call so it treats
res.err as an error (use message.error) and only calls refreshList on successful
deletion, and add a .catch to handle promise rejections (displaying err.message
or String(err) via message.error). Locate the block that calls
deleteSubscribes({ ids: [record.id] }, record.group_id) and replace the callback
with logic that checks if (res.err) -> message.error(...), else ->
message.success(t('common:success.delete')) and refreshList(), plus append
.catch(err => message.error(err.message || String(err))).
---
Duplicate comments:
In `@src/pages/alertRules/List/ListNG.tsx`:
- Around line 274-276: The operations column's render callback currently
declares a single parameter (render: (record: any) => { ... }) but Ant Design
table passes (text, record, index), so the row object should be taken from the
second parameter; change the signature to accept (text, record) or (value,
record, index) and update all uses inside the callback (e.g., accesses like
record.id, record.cate, _.get(record, [...])) to reference the second parameter
`record` rather than the first positional arg; update the render declaration
(the function used in the column definition) accordingly so runtime lookups for
record.id and record.cate work correctly.
In `@src/pages/datasource/components/TableSource/index.tsx`:
- Around line 209-216: The status-toggle and delete actions call
updateDataSourceStatus and deleteDataSource but only handle the success path;
wrap these async calls with proper rejection handling (use .catch or async/await
try/catch) so failures show a visible error (call message.error with a
descriptive message and the error) and ensure setRefresh((old) => !old) is only
invoked on successful completion; target the updateDataSourceStatus and
deleteDataSource call sites and ensure you use record.id/record.status for
context in the messages.
In `@src/pages/notificationRules/pages/List.tsx`:
- Around line 218-220: The edit menu item always links to
`/${NS}/edit/${record.id}` but must follow the same IS_PLUS branching as the
name link; update the TableActionLink (in List.tsx) to build its `to`/pathname
using the same conditional/utility used by the name link (check the name link's
implementation and the IS_PLUS flag) so that when IS_PLUS is true it navigates
to the PLUS-specific route and otherwise to the standard `edit` route, keeping
the route variable references (NS, record.id) consistent.
- Around line 235-238: The onOk handler for the delete confirm currently calls
deleteItems([record.id]).then(() => fetchData()) with no error handling; wrap
the async call in a try/catch (or add a .catch) so rejections are handled: call
deleteItems([record.id]) inside an async function or promise chain, on success
call fetchData(), on error log the error and surface it to the user (e.g.,
message.error or notification.error) and clear any loading state. Update the
onOk callback that references deleteItems and fetchData to follow the existing
error-handling pattern used elsewhere in this file.
In `@src/pages/recordingRules/PageTable.tsx`:
- Around line 244-247: The deleteRecordingRule call in the row delete handler
currently only handles success and lacks failure handling; update the async flow
around deleteRecordingRule([record.id], record.group_id) to catch errors (use
.catch or async/try-catch) and on failure call message.error(...) with an
appropriate translated error message and optionally log the error, while
preserving the existing message.success(...) and refreshList() on success;
ensure you reference the same identifiers (deleteRecordingRule, record.id,
record.group_id, message.success, refreshList) so the behavior is consistent
with other async request patterns in the codebase.
In `@src/pages/taskTpl/index.tsx`:
- Around line 196-203: The onOk handler for template deletion currently only
handles the success path; wrap the request call in proper error handling so
failures are handled deterministically: in the onOk of the delete confirm, call
request(`${api.tasktpl(record.group_id)}/${record.id}`, { method: 'DELETE' })
and add a .catch (or use async/await with try/catch) to log or surface the error
(e.g., message.error with the error message) and ensure refresh() is only called
on success; reference the onOk handler, request function, api.tasktpl(...) and
refresh() when locating where to add this handling.
In `@src/pages/user/business.tsx`:
- Around line 98-102: The confirm onOk callback currently calls
deleteBusinessTeamMember(teamId, params) and only handles the success path; wrap
the async call in a try/catch (or append .catch) so failures are handled: call
message.error(...) with a clear failure message and optionally log the error,
and ensure getTeamList() only runs on success; update the onOk handler around
deleteBusinessTeamMember and getTeamList to reflect this error handling.
In `@src/pages/variableConfigs/index.tsx`:
- Around line 173-177: The delete confirmation's onOk currently calls
deleteVariableConfigs(record.id) without handling rejections; update the onOk
handler to catch errors from deleteVariableConfigs (either by making onOk async
and using try/catch around await deleteVariableConfigs(record.id) or by
appending .catch(...) to the promise), show an error message via message.error
(using t for localization) on failure, and keep the existing success path
(message.success and fetchData()) in the success branch; reference the onOk
callback, deleteVariableConfigs, record.id, message.success, message.error, t,
and fetchData when making the change.
🪄 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: 2b920ae9-4687-4246-982e-ecb3ec5fc8ee
📒 Files selected for processing (31)
src/pages/aiConfig/agents/pages/List.tsxsrc/pages/aiConfig/llmConfigs/pages/List.tsxsrc/pages/aiConfig/mcpServers/pages/List.tsxsrc/pages/aiConfig/skills/pages/ResourcesTable.tsxsrc/pages/alertCurEvent/pages/List/AlertTable.tsxsrc/pages/alertRules/List/ListNG.tsxsrc/pages/builtInComponents/AlertRules/index.tsxsrc/pages/builtInComponents/CollectTpls/index.tsxsrc/pages/builtInComponents/Dashboards/index.tsxsrc/pages/builtInComponents/Metrics/index.tsxsrc/pages/dashboard/List/index.tsxsrc/pages/datasource/components/TableSource/index.tsxsrc/pages/embeddedProduct/pages/List/index.tsxsrc/pages/event/Table.tsxsrc/pages/event/card.tsxsrc/pages/eventPipeline/pages/List/index.tsxsrc/pages/historyEvents/ListNG/index.tsxsrc/pages/log/IndexPatterns/index.tsxsrc/pages/metricsBuiltin/List.tsxsrc/pages/notificationChannels/pages/List/index.tsxsrc/pages/notificationChannels/pages/ListNG/index.tsxsrc/pages/notificationRules/pages/List.tsxsrc/pages/recordingRules/PageTable.tsxsrc/pages/task/index.tsxsrc/pages/taskTpl/index.tsxsrc/pages/user/business.tsxsrc/pages/user/groups.tsxsrc/pages/user/users.tsxsrc/pages/variableConfigs/index.tsxsrc/pages/warning/shield/index.tsxsrc/pages/warning/subscribe/ListNG.tsx
Comment on lines
+199
to
+203
| deleteMetrics([record.id]).then(() => { | ||
| message.success(t('common:success.delete')); | ||
| setRefreshFlag(_.uniqueId('refreshFlag_')); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Add failure handling for single-row delete.
This delete path is success-only; failed requests currently produce no user feedback.
Suggested patch
onOk() {
- deleteMetrics([record.id]).then(() => {
- message.success(t('common:success.delete'));
- setRefreshFlag(_.uniqueId('refreshFlag_'));
- });
+ return deleteMetrics([record.id])
+ .then(() => {
+ message.success(t('common:success.delete'));
+ setRefreshFlag(_.uniqueId('refreshFlag_'));
+ })
+ .catch((err) => {
+ message.error(err?.message || t('common:error.delete'));
+ return Promise.reject(err);
+ });
},📝 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
| deleteMetrics([record.id]).then(() => { | |
| message.success(t('common:success.delete')); | |
| setRefreshFlag(_.uniqueId('refreshFlag_')); | |
| }); | |
| }, | |
| onOk() { | |
| return deleteMetrics([record.id]) | |
| .then(() => { | |
| message.success(t('common:success.delete')); | |
| setRefreshFlag(_.uniqueId('refreshFlag_')); | |
| }) | |
| .catch((err) => { | |
| message.error(err?.message || t('common:error.delete')); | |
| return Promise.reject(err); | |
| }); | |
| }, |
🤖 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/builtInComponents/Metrics/index.tsx` around lines 199 - 203, The
single-row delete call using deleteMetrics([record.id]) currently only handles
success; add error handling so failures show feedback and do not refresh. Wrap
the delete call in a proper .catch (or convert to async/await with try/catch)
around the promise returned by deleteMetrics in the same block where
message.success(...) and setRefreshFlag(_.uniqueId('refreshFlag_')) are used; on
error call message.error(...) with an appropriate translated message (use
t('common:...') or include err.message) and avoid calling setRefreshFlag when
the delete fails. Ensure you reference deleteMetrics, message.success,
message.error, setRefreshFlag and _.uniqueId in the fix.
- Add Tags component with two-line overflow layout and ResizeObserver - Use Tags in the event count column with triggered/normal status display - Add i18n entries for status text and popover title across all locales
… in hosts list Move Tags from alertRules scope to components/TableTags for cross-page reuse; replace TableTags usage in hosts list with the unified Tags component.
…y column - Refactor Tags component to support generic typed data via getKey/getLabel - Support per-item bgColor/fontColor/icon via function props - Add maxWidth/borderRadius props for layout control - Extract calcLayout and add comprehensive test suite - Move alert rule category icon to datasource column - Add severity column with color-coded per-level tags - Merge nickname column into username column - Add click-to-open notification rules with not-found warning
- Remove scroll={{ x: 'max-content' }} from all table components to
simplify layout behavior
- Replace TableTags with the new Tags component using outline style
and consistent maxWidth
- Add null-safety to Tags component (accept null data, early return)
- Split update_at into two-row layout (date + time) across all tables
- Combine update_by/update_by_nickname and create_by/create_by_nickname
into single columns with stacked display
- Inline business group name into metric name column in recordingRules
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
Test
Summary by CodeRabbit
New Features
UI/UX Improvements
Refactor