Copilot commented on code in PR #877:
URL:
https://github.com/apache/rocketmq-dashboard/pull/877#discussion_r3705323399
##########
web/src/pages/studio/SslSettings.tsx:
##########
@@ -66,8 +66,7 @@ interface FormValues {
// ─── Component ──────────────────────────────────────────────────
const SslSettingsPage = () => {
const [form] = Form.useForm<FormValues>();
- const [loading, setLoading] = useState(false);
- const [sslConfig, setSslConfig] = useState<SslConfig>({
+ const [sslConfig] = useState<SslConfig>({
enabled: false,
protocol: 'TLSv1.3',
keyStoreType: 'JKS',
Review Comment:
`sslConfig` is stored in React state but never updated (the setter was
removed). Since it’s effectively constant config used for reset defaults,
consider replacing `useState` with a plain constant (or a module-level
constant) to avoid implying mutability and simplify future maintenance.
##########
web/src/pages/studio/__tests__/SslSettings.test.tsx:
##########
@@ -124,6 +124,26 @@ describe('SslSettings Page', () => {
expect(screen.getByRole('button', { name: /重\s*置/ })).toBeInTheDocument();
});
+ it('does not persist SSL changes when the backend API is unavailable', async
() => {
+ const user = userEvent.setup();
+ renderWithProviders(<SslSettings />);
+
+ const switchEl = screen.getByRole('switch');
+ await user.click(switchEl);
+ await user.type(screen.getByLabelText('KeyStore 路径'),
'/etc/rocketmq/keystore.jks');
+ await user.type(screen.getByLabelText('KeyStore 密码'), 'changeit');
Review Comment:
This test hard-codes full localized UI strings and uses `user.type` without
clearing first. That can make it brittle if the copy changes or if the fields
have non-empty defaults. Consider (a) matching with a smaller regex subset of
the message or using the i18n key-driven value in assertions, and (b) clearing
inputs before typing to avoid accidental appends.
##########
web/src/i18n/translations.ts:
##########
@@ -1057,6 +1057,10 @@ const translations: Record<string, Record<Lang, string>>
= {
'ssl.expiryDate': { zh: '过期日期', en: 'Expiry Date' },
'ssl.active': { zh: '有效', en: 'Active' },
'ssl.saveSuccess': { zh: 'SSL 配置保存成功', en: 'SSL configuration saved
successfully' },
+ 'ssl.saveUnavailable': {
+ zh: 'SSL 配置保存功能尚未接入真实后端接口',
+ en: 'SSL configuration persistence is not wired to a backend API yet',
+ },
Review Comment:
The English string reads a bit implementation-internal (“wired”) and may be
unclear to end users. Consider a more user-facing phrasing (e.g., “Saving SSL
configuration isn’t available until the server API is implemented.”) to reduce
confusion.
##########
web/src/pages/studio/__tests__/SslSettings.test.tsx:
##########
@@ -124,6 +124,26 @@ describe('SslSettings Page', () => {
expect(screen.getByRole('button', { name: /重\s*置/ })).toBeInTheDocument();
});
+ it('does not persist SSL changes when the backend API is unavailable', async
() => {
+ const user = userEvent.setup();
+ renderWithProviders(<SslSettings />);
+
+ const switchEl = screen.getByRole('switch');
+ await user.click(switchEl);
+ await user.type(screen.getByLabelText('KeyStore 路径'),
'/etc/rocketmq/keystore.jks');
+ await user.type(screen.getByLabelText('KeyStore 密码'), 'changeit');
+
+ await user.click(screen.getByRole('button', { name: /保\s*存/ }));
+
+ expect(await screen.findByText('SSL
配置保存功能尚未接入真实后端接口')).toBeInTheDocument();
+ expect(screen.queryByText('SSL 配置保存成功')).not.toBeInTheDocument();
Review Comment:
This test hard-codes full localized UI strings and uses `user.type` without
clearing first. That can make it brittle if the copy changes or if the fields
have non-empty defaults. Consider (a) matching with a smaller regex subset of
the message or using the i18n key-driven value in assertions, and (b) clearing
inputs before typing to avoid accidental appends.
--
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]
