Aias00 commented on PR #638:
URL:
https://github.com/apache/rocketmq-dashboard/pull/638#issuecomment-5138729468
> Thanks for the PR! Three of the four commits look good (load page data
after mount, preserve topic table pagination, use auth admin flag for Ops
writes). However the consumer-page commit ("Wire consumer page to group APIs")
now conflicts in design with #631, which was merged first and already routes
the consumer page through the service layer:
>
> 1. **Duplicate implementations**: #631 loads detail via separate
`loadSubscriptions` / `loadProgress` helpers with a `subscriptionsByGroup`
cache, while this PR introduces a parallel `Promise.all` + `detailLoading` /
`normalizeGroup` / `buildSubscriptionFallback` approach for the same feature.
Merging both would replace the already-merged design rather than build on it.
> 2. **Hardcoded Chinese error messages**: strings like `消费组详情加载失败，请稍后重试`
should go through the i18n layer (same feedback as #518).
>
> Could you rebase onto the latest `rocketmq-studio` and rework the
consumer-page commit on top of #631's structure (keeping the useful additions
like `submitting` / `resetSubmitting` states), with error messages localized?
The other three commits should then apply cleanly. Happy to review again after
the update.
The duplicated consumer-page implementation mentioned in the review is gone,
and the page now builds on the #631 structure. The hardcoded Chinese error
messages were also moved to i18n keys.
--
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]
