utafrali commented on code in PR #7364:
URL:
https://github.com/apache/rocketmq-dashboard/pull/7364#discussion_r4387268084
##########
web/src/pages/instance/topic.tsx:
##########
@@ -1330,9 +1364,22 @@ const TopicPageContent = ({
danger
icon={<DeleteOutlined />}
onClick={() => {
+ const liteParentCount = filteredTopics.filter(
+ (topic) => selectedRowKeys.includes(topic.name) &&
topic.type === 'LITE',
Review Comment:
The Lite-parent count is derived from `filteredTopics`, but
`selectedRowKeys` persists across filter changes in Ant Design tables. If a
user selects rows (including Lite parents), then applies a search filter that
hides those Lite parents, `filteredTopics` will not contain them —
`liteParentCount` comes out 0 and the cascade warning is silently skipped even
though the subsequent delete on line 1389 will still cascade.
A safer approach is to derive the count from the stable topic data source
rather than the current filtered view:
```tsx
const liteParentCount = topics.filter(
(topic) => selectedRowKeys.includes(topic.name) && topic.type === 'LITE',
).length;
```
(or whichever variable holds the unfiltered loaded dataset). Alternatively,
clearing the selection when filters change also fixes this, but the
count-from-source approach is lower-friction.
##########
web/src/pages/instance/topic.tsx:
##########
@@ -669,26 +670,59 @@ const TopicPageContent = ({
sendForm.setFieldsValue({ topic: topic.name, tag: '', key: '', body: '',
properties: [] });
setSendModalOpen(true);
} else if (key === 'delete') {
- modal.confirm({
- title: '确认删除',
- content: `确定要删除 Topic「${topic.name}」吗？此操作不可撤销。`,
- okText: '删除',
- okType: 'danger',
- cancelText: '取消',
- onOk: async () => {
- try {
- await deleteTopic(topic.name, selectedInstanceId || undefined);
- // Drop the deleted row from the selection: a checked row that
disappears would
- // otherwise keep the batch delete armed with a name that no
longer exists, and its
- // failure path re-seeds that same selection - leaving nothing to
uncheck.
- setSelectedRowKeys((previous) => previous.filter((key) => key !==
topic.name));
- await reloadTopicPage();
- message.success(`Topic「${topic.name}」已删除`);
- } catch {
- message.error('删除 Topic 失败，请稍后重试');
- }
- },
- });
+ const openDeleteConfirm = (liteSummary: LiteTopicItem | undefined) => {
+ modal.confirm({
+ title: '确认删除',
Review Comment:
The surrounding confirm dialog still uses hardcoded Chinese for `title`,
`okText`, `cancelText`, success, and error messages (lines 675, 695–697, 706,
708) while the new Lite-specific content added here correctly goes through
`t()`. Since this PR already touches every one of these lines, extracting the
remaining hardcoded strings into `translations.ts` would leave the file
internally consistent. Not a blocker, but easy to do in the same pass.
##########
web/src/pages/studio/LiteTopic.tsx:
##########
@@ -1063,6 +1145,51 @@ const LiteTopicPage: React.FC = () => {
</Form.Item>
</Form>
</Modal>
+
+ {/* Delete Lite parent Modal */}
+ <Modal
+ title={t('liteTopic.deleteModalTitle')}
+ open={deleteModalOpen}
+ onOk={handleDeleteParent}
+ onCancel={closeDelete}
+ confirmLoading={deleteLoading}
+ okText={t('liteTopic.deleteParent')}
+ okButtonProps={{ danger: true }}
+ cancelText={t('common.cancel')}
+ destroyOnHidden
+ >
+ <Form layout="vertical" style={{ marginTop: 16 }}>
+ <div style={{ marginBottom: 16 }}>
+ <div>{t('topic.liteDeleteWarning', { name:
deleteTarget?.topicPattern || '' })}</div>
+ {deleteTarget?.topicCount != null && deleteTarget?.consumerCount
!= null && (
Review Comment:
The delete modal shows `topicCount` / `consumerCount` from the record that
was in the table at the last list refresh — potentially stale if the user has
been sitting on the page. In contrast, `topic.tsx` makes a fresh
`queryLiteTopicList` call on every single-delete to show real-time counts. The
two paths are asymmetric.
For most workflows this is fine (the list just loaded), but if stale
blast-radius numbers matter, consider doing the same fresh lookup here. If the
decision to skip the round-trip is intentional, a short comment explaining the
trade-off would help reviewers later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
