RockteMQ-AI commented on code in PR #1177:
URL:
https://github.com/apache/rocketmq-dashboard/pull/1177#discussion_r3733041793
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/broker/RuntimeAdminClientResolver.java:
##########
@@ -20,19 +20,30 @@ public class RuntimeAdminClientResolver {
private final InstanceRepository instanceRepository;
private final MqAdminExtFactory adminFactory;
- public String resolveEndpoint(String instanceId) {
+ public InstanceVO resolveInstance(String instanceId) {
if (!StringUtils.hasText(instanceId)) {
throw new BusinessException(400, "instanceId is required");
}
- InstanceVO instance = instanceRepository.findById(instanceId)
+ return instanceRepository.findById(instanceId)
.orElseThrow(() -> new BusinessException(404, "Instance not
found: " + instanceId));
+ }
+
+ public String resolveEndpoint(String instanceId) {
+ InstanceVO instance = resolveInstance(instanceId);
if (!StringUtils.hasText(instance.getEndpoint())) {
throw new BusinessException(400, "Instance has no endpoint: " +
instanceId);
}
return instance.getEndpoint().trim();
}
public <T> T execute(String instanceId, MqAdminExtFactory.AdminAction<T>
action) {
- return adminFactory.execute(resolveEndpoint(instanceId), null, action);
+ return execute(resolveInstance(instanceId), action);
+ }
+
+ public <T> T execute(InstanceVO instance, MqAdminExtFactory.AdminAction<T>
action) {
+ if (instance == null || !StringUtils.hasText(instance.getEndpoint())) {
Review Comment:
**[Info]** Minor inconsistency: the new `execute(InstanceVO, ...)` overload
trims the endpoint (`instance.getEndpoint().trim()`), but `resolveEndpoint()`
returns the raw value without trimming. Consider applying `.trim()` in
`resolveEndpoint()` as well for consistency, or document the difference.
##########
web/src/pages/settings/index.tsx:
##########
@@ -380,6 +397,17 @@ export const DataSourceTab = () => {
render: (t: string) => <Tag color={typeTagColor[t]}>{t}</Tag>,
},
{ title: 'URL', dataIndex: 'url', key: 'url' },
+ {
Review Comment:
**[Info]** The new column title and form label/placeholder are in Chinese
only. If the app supports English locale, consider adding these to the i18n
files. (This may be consistent with existing patterns on this page.)
##########
web/src/pages/home/dashboard.tsx:
##########
@@ -18,29 +20,68 @@ const DashboardPage = () => {
const navigate = useNavigate();
const { t } = useLang();
const [dashboard, setDashboard] = useState<DashboardData | null>(null);
+ const [instances, setInstances] = useState<Instance[]>([]);
+ const [selectedInstanceId, setSelectedInstanceId] = useState<string>();
const [loading, setLoading] = useState(true);
const [loadError, setLoadError] = useState(false);
const loadDashboard = useCallback(async () => {
setLoading(true);
setLoadError(false);
try {
- setDashboard(await getDashboard());
+ setDashboard(await getDashboard(selectedInstanceId));
} catch {
setLoadError(true);
} finally {
setLoading(false);
}
+ }, [selectedInstanceId]);
+
+ useEffect(() => {
+ let cancelled = false;
+ void listInstances()
+ .then((nextInstances) => {
+ if (!cancelled) setInstances(nextInstances);
+ })
+ .catch(() => {
+ if (!cancelled) setInstances([]);
+ });
+ return () => {
+ cancelled = true;
+ };
}, []);
useEffect(() => {
void Promise.resolve().then(loadDashboard);
}, [loadDashboard]);
+ const dashboardHeader = (
+ <PageHeader
+ title={t('dashboard.title')}
Review Comment:
**[Info]** The `placeholder` ("All configured instances") and the Refresh
button text are hardcoded in English. The rest of the dashboard uses `t()` for
i18n. Consider adding these strings to the locale files for consistency with
the rest of the UI.
##########
web/src/components/MetricsExplorer.tsx:
##########
@@ -365,6 +377,13 @@ const MetricsExplorer = () => {
};
}, []);
+ useEffect(() => {
Review Comment:
**[Info]** The `useEffect` that resets `dataSourceKey` when the current
selection becomes unavailable is correct. Note that `setDataSourceKey` and
`setData` are intentionally omitted from the dependency array since React
guarantees their stability — this is fine but may trigger an ESLint
`exhaustive-deps` warning if the rule is enabled.
--
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]
