You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Aias00 (via GitHub)" <gi...@apache.org> on 2026/08/03 15:09:06 UTC
[PR] fix(web): stop simulating SSL settings saves [rocketmq-dashboard]
Aias00 opened a new pull request, #877:
URL: https://github.com/apache/rocketmq-dashboard/pull/877
## Summary
- remove the fake setTimeout-based SSL settings save success path
- show an explicit unavailable message until a real backend SSL configuration API exists
- add a regression test that verifies SSL edits are not persisted locally after a failed save
Fixes #876
## Tests
- npm test -- --run src/pages/studio/__tests__/SslSettings.test.tsx
- npm run lint -- src/pages/studio/SslSettings.tsx src/pages/studio/__tests__/SslSettings.test.tsx src/i18n/translations.ts
- git diff --check
Note: lint reports only the existing react-refresh warnings in LangContext.tsx, main.tsx, and ThemeContext.tsx.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(web): stop simulating SSL settings saves [rocketmq-dashboard]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 closed pull request #877: fix(web): stop simulating SSL settings saves
URL: https://github.com/apache/rocketmq-dashboard/pull/877
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(web): stop simulating SSL settings saves [rocketmq-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(web): stop simulating SSL settings saves [rocketmq-dashboard]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #877:
URL: https://github.com/apache/rocketmq-dashboard/pull/877#issuecomment-5175212990
Superseded by consolidated PR #938, which contains this change and has passing checks. Closing this narrower PR to keep review focused.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org