anaylab commented on PR #725: URL: https://github.com/apache/shenyu-dashboard/pull/725#issuecomment-6295175721
Thanks for the fix, @bhaskargurram-ai. The approach looks right to me: calling `queryAnalysis()` on namespace change, tracking `analysisNamespaceId`, and dropping responses for a namespace that's no longer selected. I read through the diff and have a few small notes, none of them blocking. 1. **`lineList` vs `lineData`.** The initial state declares `lineList`, but the reducer and effects use `lineData`. This was already there before the PR, so it's fine to leave, but renaming it would remove the mismatch. If you do, it's worth checking that `renderLineChart` copes with `lineData` being `undefined` before the first load. 2. **Stale check in `fetchAnalysis`.** I agree with Copilot that the clear runs before any stale check. In practice dva processes the action synchronously on dispatch, so I couldn't construct a sequence (A→B→C, A→B→A) that ends in the wrong state. An early guard against `global.currentNamespaceId` would still make the intent clearer. 3. **Rendering with empty data.** The tests cover the model and the dispatches. Since the description says the charts show "no data" while loading, it would be good to confirm `renderPieChart` and `renderLineChart` handle empty arrays cleanly, ideally with a quick manual check in a browser. Same-namespace refreshes arriving out of order still aren't guarded, but that was already true before this PR and is probably outside its scope. -- 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]
