Skip to content

fix(home): guard and stabilize dashboard number rendering - #2875

Open
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-home-dashnum
Open

fix(home): guard and stabilize dashboard number rendering#2875
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-home-dashnum

Conversation

@yyqdbngt

Copy link
Copy Markdown

Summary

  • Consolidate dashboard number rendering into an exported renderCount() helper: null/undefined/NaN become the existing N/A placeholder, and formatting uses a stable en-US locale (same convention as formatNumber in utils/format.ts)
  • Apply it to the broker/proxy/TPS-in/TPS-out table columns and the TPS-out stat card detail
  • Guard the "million messages" stat detail against a non-finite totalMessagesToday (renders 0 instead of NaN)
  • Pass a guaranteed array to MiniBar when a cluster row omits throughput
  • Add regression tests: a cluster with null TPS values and a missing throughput series renders without crashing, plus unit tests for renderCount

Why

Dashboard data comes from the backend over JSON, so field shapes are not guaranteed even when the TypeScript type says otherwise. Two crash/leak paths: v.toLocaleString() on the TPS columns throws for a null value taken from the wire, and MiniBar reads data.length, so a cluster row missing throughput crashed the whole dashboard page (no error boundary). The unqualified toLocaleString() calls additionally produced locale-dependent separators that varied with the viewer's browser locale, inconsistent with the rest of the app's stable formatting.

Testing

  • ./node_modules/.bin/vitest run src/pages/home/ → 12 passed (2 new)
  • ./node_modules/.bin/tsc --noEmit → clean
  • ./node_modules/.bin/eslint src/pages/home/dashboard.tsx src/pages/home/__tests__/DashboardPage.test.tsx → 0 errors

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR contains moderate changes (135 lines). I have performed an initial structural review.

Observations

  • Changes appear well-scoped
  • CLA status: unknown

Maintainers: please verify the implementation details match the intended behavior.


Automated review by github-manager-bot

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Defensive fix that improves input validation and error handling. Code looks clean and follows existing patterns.

LGTM


Automated review by "github-manager-bot"

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — defensive fix improving input validation and error handling.


Automated review by "github-manager-bot"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants