This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new 036678011 fix(alerts): isolate editor state between alert domains
(#6190)
036678011 is described below
commit 0366780111491a0e42b2fbc550167eb98005e6a0
Author: zmuxuny <[email protected]>
AuthorDate: Mon Oct 12 00:18:15 2026 -0700
fix(alerts): isolate editor state between alert domains (#6190)
`/ops/alerts` and `/ops/business-alerts` render the same component at the
same router position, so React reuses one instance across a domain change. The
render-phase reset at `alerts.tsx:176-192` dropped list state only — page,
search, enabled filter, selection, rows, total, runtime, loading — and left
`modalVisible`, `editingRule`, the form values, `testResult`, `submitting`,
`bulkAction` and `metricRequestVersion` in place. Going back from CLUSTER to
BUSINESS with an editor open the [...]
Three new cases cover the change. Removing `key={domain}` leaves the
previous domain's editor open in `closes the previous domain editor when
browser history switches alert routes` and lets the stale result through in
`ignores a previous domain test result after a new editor has opened`. `keeps
unsaved editor inputs across navigation within the same alert domain` is the
control that fails if the key is made to change on every render, and the
pre-existing `resets page, search and statu [...]
---
web/src/pages/ops/__tests__/AlertsPage.test.tsx | 138 +++++++++++++++++++++++-
web/src/pages/ops/alerts.tsx | 26 ++---
2 files changed, 144 insertions(+), 20 deletions(-)
diff --git a/web/src/pages/ops/__tests__/AlertsPage.test.tsx
b/web/src/pages/ops/__tests__/AlertsPage.test.tsx
index d9b5fe759..5b71009ff 100644
--- a/web/src/pages/ops/__tests__/AlertsPage.test.tsx
+++ b/web/src/pages/ops/__tests__/AlertsPage.test.tsx
@@ -16,10 +16,16 @@
*/
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from
'vitest';
-import { cleanup, fireEvent, render, screen, waitFor, within } from
'@testing-library/react';
+import { act, cleanup, fireEvent, render, screen, waitFor, within } from
'@testing-library/react';
import userEvent from '@testing-library/user-event';
import { App } from 'antd';
-import type { AlertRule, NativeAlertMetricInfo, PageResult } from
'../../../api/ops';
+import { MemoryRouter, Route, Routes, useNavigate } from 'react-router-dom';
+import type {
+ AlertRule,
+ AlertRuleTestResult,
+ NativeAlertMetricInfo,
+ PageResult,
+} from '../../../api/ops';
import { LangProvider } from '../../../i18n/LangContext';
import { LANGUAGE_STORAGE_KEY } from '../../../i18n/languagePreference';
import { formatUtcDateTime } from '../../../utils/format';
@@ -34,6 +40,8 @@ import {
listNativeAlertMetrics,
importAlertRulesTransfer,
toggleAlertRule,
+ updateAlertRule,
+ testAlertRule,
} from '../../../services/opsService';
vi.mock('../../../services/instanceService', () => ({
@@ -52,6 +60,7 @@ vi.mock('../../../services/opsService', () => ({
exportAlertRulesTransfer: vi.fn(),
importAlertRulesTransfer: vi.fn(),
updateAlertRule: vi.fn(),
+ testAlertRule: vi.fn(),
}));
const alertRules: AlertRule[] = [
@@ -133,6 +142,46 @@ function renderPage(domain?: 'BUSINESS' | 'CLUSTER') {
);
}
+function renderAlertHistory() {
+ const HistoryControls = () => {
+ const navigate = useNavigate();
+ return (
+ <>
+ <button onClick={() => navigate(-1)}>Browser back</button>
+ <button onClick={() => navigate(1)}>Browser forward</button>
+ <button onClick={() => navigate('/ops/alerts?view=details')}>Same
domain query</button>
+ </>
+ );
+ };
+ return render(
+ <App>
+ <LangProvider>
+ <MemoryRouter initialEntries={['/ops/business-alerts', '/ops/alerts']}
initialIndex={1}>
+ <HistoryControls />
+ <Routes>
+ <Route path="/ops/alerts" element={<AlertsPage />} />
+ <Route path="/ops/business-alerts" element={<AlertsPage
domain="BUSINESS" />} />
+ </Routes>
+ </MemoryRouter>
+ </LangProvider>
+ </App>,
+ );
+}
+
+function mockDomainRules() {
+ vi.mocked(listAlertRulesPage).mockImplementation(async (domain) =>
+ pageResult([
+ {
+ ...cloneRule(alertRules[0]),
+ id: domain === 'BUSINESS' ? 10 : 1,
+ name: domain === 'BUSINESS' ? 'Business rule' : 'Cluster rule',
+ instanceId: 'local',
+ metric: 'broker.disk.usage_ratio',
+ },
+ ]),
+ );
+}
+
function getRuleRow(ruleName: string) {
const row = screen.getByText(ruleName).closest('tr');
if (!row) throw new Error(`Row not found: ${ruleName}`);
@@ -372,6 +421,91 @@ describe('AlertsPage', () => {
expect(await screen.findByText('共 21 条规则')).toBeInTheDocument();
});
+ it('closes the previous domain editor when browser history switches alert
routes', async () => {
+ mockDomainRules();
+ vi.mocked(updateAlertRule).mockImplementation(async (data) => ({
+ ...cloneRule(alertRules[0]),
+ ...data,
+ }));
+ renderAlertHistory();
+ await screen.findByText('Cluster rule');
+ await expectRuleRowInteractive('Cluster rule');
+ fireEvent.click(within(getRuleRow('Cluster rule')).getByRole('button', {
name: '编辑' }));
+ expect(await screen.findByRole('dialog', { name: '编辑'
})).toBeInTheDocument();
+ fireEvent.click(screen.getByRole('button', { name: 'Browser back' }));
+ await screen.findByText('Business rule');
+ expect(screen.queryByRole('dialog', { name: '编辑'
})).not.toBeInTheDocument();
+ expect(updateAlertRule).not.toHaveBeenCalled();
+ fireEvent.click(within(getRuleRow('Business rule')).getByRole('button', {
name: '编辑' }));
+ const businessEditor = await screen.findByRole('dialog', { name: '编辑' });
+ fireEvent.click(within(businessEditor).getByRole('button', { name: /编\s*辑/
}));
+ await waitFor(() =>
+ expect(updateAlertRule).toHaveBeenCalledWith(
+ expect.objectContaining({ id: 10, name: 'Business rule' }),
+ 'BUSINESS',
+ ),
+ );
+ fireEvent.click(screen.getByRole('button', { name: 'Browser forward' }));
+ await screen.findByText('Cluster rule');
+ expect(screen.queryByRole('dialog', { name: '编辑'
})).not.toBeInTheDocument();
+ });
+
+ it('keeps unsaved editor inputs across navigation within the same alert
domain', async () => {
+ mockDomainRules();
+ renderAlertHistory();
+ await screen.findByText('Cluster rule');
+ await expectRuleRowInteractive('Cluster rule');
+ fireEvent.click(within(getRuleRow('Cluster rule')).getByRole('button', {
name: '编辑' }));
+ const dialog = await screen.findByRole('dialog', { name: '编辑' });
+ fireEvent.change(within(dialog).getByDisplayValue('Cluster rule'), {
+ target: { value: 'Unsaved rule name' },
+ });
+ fireEvent.click(screen.getByRole('button', { name: 'Same domain query' }));
+ expect(within(dialog).getByDisplayValue('Unsaved rule
name')).toBeInTheDocument();
+ fireEvent.click(screen.getByRole('button', { name: 'Browser back' }));
+ expect(within(dialog).getByDisplayValue('Unsaved rule
name')).toBeInTheDocument();
+ fireEvent.click(within(dialog).getByRole('button', { name: /取\s*消/ }));
+ fireEvent.click(within(getRuleRow('Cluster rule')).getByRole('button', {
name: '编辑' }));
+ const reopened = await screen.findByRole('dialog', { name: '编辑' });
+ expect(within(reopened).getByDisplayValue('Cluster
rule')).toBeInTheDocument();
+ });
+
+ it('ignores a previous domain test result after a new editor has opened',
async () => {
+ mockDomainRules();
+ let completeTest!: (result: AlertRuleTestResult) => void;
+ vi.mocked(testAlertRule).mockImplementationOnce(
+ () =>
+ new Promise((resolve) => {
+ completeTest = resolve;
+ }),
+ );
+ renderAlertHistory();
+ await screen.findByText('Cluster rule');
+ await expectRuleRowInteractive('Cluster rule');
+ fireEvent.click(within(getRuleRow('Cluster rule')).getByRole('button', {
name: '编辑' }));
+ const dialog = await screen.findByRole('dialog', { name: '编辑' });
+ fireEvent.click(within(dialog).getByRole('button', { name: '试运行' }));
+ await waitFor(() => expect(testAlertRule).toHaveBeenCalledTimes(1));
+ fireEvent.click(screen.getByRole('button', { name: 'Browser back' }));
+ await screen.findByText('Business rule');
+ fireEvent.click(within(getRuleRow('Business rule')).getByRole('button', {
name: '编辑' }));
+ const nextDialog = await screen.findByRole('dialog', { name: '编辑' });
+ await act(async () =>
+ completeTest({
+ samples: [
+ {
+ labels: { broker: 'previous-cluster' },
+ availability: 'AVAILABLE',
+ currentValue: 90,
+ conditionMet: true,
+ },
+ ],
+ }),
+ );
+
expect(within(nextDialog).queryByText('broker=previous-cluster')).not.toBeInTheDocument();
+ expect(within(nextDialog).getByDisplayValue('Business
rule')).toBeInTheDocument();
+ });
+
it('resets page, search and status filters when the domain switches', async
() => {
vi.mocked(listAlertRulesPage).mockClear();
vi.mocked(listAlertRulesPage).mockResolvedValue({
diff --git a/web/src/pages/ops/alerts.tsx b/web/src/pages/ops/alerts.tsx
index 3ce1f9357..cd40b629b 100644
--- a/web/src/pages/ops/alerts.tsx
+++ b/web/src/pages/ops/alerts.tsx
@@ -152,7 +152,7 @@ interface AlertsPageProps {
domain?: AlertRuleDomain;
}
-const AlertsPage = ({ domain = 'CLUSTER' }: AlertsPageProps) => {
+const AlertsPageContent = ({ domain = 'CLUSTER' }: AlertsPageProps) => {
const { t } = useLang();
const { token } = theme.useToken();
const [rules, setRules] = useState<AlertRule[]>([]);
@@ -173,23 +173,6 @@ const AlertsPage = ({ domain = 'CLUSTER' }:
AlertsPageProps) => {
const [selectedRuleIds, setSelectedRuleIds] = useState<Key[]>([]);
const [bulkAction, setBulkAction] = useState<'enable' | 'disable' | 'delete'
| null>(null);
const [form] = Form.useForm();
- // The same component instance serves /ops/alerts and /ops/business-alerts,
so the
- // list state from the previous domain must be dropped when the route
switches
- // (React's documented "adjust state when a prop changes" pattern). Clear
the loaded
- // rows/total/runtime and re-enter the loading state as well, otherwise the
previous
- // domain's rules stay visible with loading=false until the new page
resolves.
- const [renderedDomain, setRenderedDomain] = useState(domain);
- if (renderedDomain !== domain) {
- setRenderedDomain(domain);
- setPage(1);
- setSearch('');
- setEnabledFilter(undefined);
- setSelectedRuleIds([]);
- setRules([]);
- setTotalRules(0);
- setRuntime([]);
- setLoading(true);
- }
const selectedMetric = Form.useWatch('metric', form);
const selectedOperator = Form.useWatch('operator', form);
const selectedThresholdUnit = Form.useWatch('thresholdUnit', form);
@@ -1533,4 +1516,11 @@ const AlertsPage = ({ domain = 'CLUSTER' }:
AlertsPageProps) => {
);
};
+// Route changes reuse this page component. A domain owns its editor, metric
requests,
+// test results and mutation state as well as its list; none may leak into the
other domain.
+// The stable key preserves edits during navigation that stays within the same
domain.
+const AlertsPage = ({ domain = 'CLUSTER' }: AlertsPageProps) => (
+ <AlertsPageContent key={domain} domain={domain} />
+);
+
export default AlertsPage;
