fix(events): Sum chart tooltip showing raw floating-point value - #7846
Open
LaNinja06 wants to merge 3 commits into
Open
fix(events): Sum chart tooltip showing raw floating-point value#7846LaNinja06 wants to merge 3 commits into
LaNinja06 wants to merge 3 commits into
Conversation
The Events segmentation chart tooltip could show a sum like
303.35999999999996 instead of 303.36, while the summary tiles/table
on the same page correctly rounded it. Root cause: the chart series
pushed the raw accumulated float, and the tooltip's default formatter
(getShortNumber) doesn't round for values in [0.1, 10000).
- Round the sum series to 2 decimals at construction in
getLineChartData and getBarChartData (matching the tile/table
convention).
- Fix a pre-existing bug in getShortNumber where
.replace(".0", "") could corrupt values like 303.06 into "3036"
by stripping the first mid-string ".0"; anchor to /\.0$/ so only a
genuinely trailing ".0" (K/M/B abbreviations) is stripped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent numeric formatting in the Events segmentation chart tooltip by rounding sum-series values to 2 decimals when constructing chart series data, aligning tooltip values with the existing tile/table rounding behavior. It also corrects a string-replacement bug in getShortNumber that could remove a mid-string .0 sequence and corrupt certain values.
Changes:
- Round the Events “sum” series values to 2 decimals when building line and bar chart series data.
- Fix
getShortNumberto only strip a trailing.0viareplace(/\.0$/, ""), preventing mid-string corruption (e.g.,303.06).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/express/public/javascripts/countly/countly.common.js | Fixes getShortNumber to only remove a trailing .0, preventing incorrect value corruption. |
| frontend/express/public/core/events/javascripts/countly.details.models.js | Rounds sum-series values to 2 decimals when building chart series data for line/bar charts to avoid raw floating-point display. |
kanwarujjaval
approved these changes
Jul 27, 2026
Addresses review feedback on the platform counterpart of this fix: showSumGraph was computed from the rounded series, so buckets whose raw sums all round to 0.00 (e.g. 0.004) would hide the sum series entirely, while the summary tiles - computed from raw chartData - still showed a non-zero total. Check the raw `s` values instead, which restores the pre-rounding visibility semantics exactly while keeping the 2-decimal rounding for display. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Events segmentation chart tooltip could show a sum like
303.35999999999996instead of303.36, while the summary tiles/table on the same page correctly rounded it. Root cause: the chart series pushed the raw accumulated float, and the tooltip's default formatter (getShortNumber) doesn't round for values in[0.1, 10000).This rounds the sum series to 2 decimals at construction (matching the existing tile/table convention), and separately fixes a pre-existing bug in
getShortNumberwhere.replace(".0", "")could corrupt values like303.06into3036by stripping the first mid-string.0instead of only a trailing one.Changes
countly.details.models.js—getLineChartDataandgetBarChartData: round thes(sum) series to 2 decimals when building chart data.countly.details.models.js—showSumGraphin both functions now derives from the rawsvalues rather than the rounded series. Without this, buckets whose raw sums all round to0.00(e.g.0.004) would hide the sum series entirely while the summary tile still showed a non-zero total.countly.common.js—getShortNumber: in the[0.1, 10000)branch only,.replace(".0", "")→.replace(/\.0$/, ""). That branch stringifies the raw float, so the digits after the decimal point are unbounded and a literal.0could match mid-string. TheK/M/Bbranches are deliberately left unchanged: they operate on.toFixed(1)output, which always has exactly one fractional digit, so.0can only ever occur at the end there.Verification
Verified against 7 cases (positive/negative sums, whole-number rounding, and the K-abbreviation boundary at 10000) — tooltip, tiles, and count/duration formatting all behave as expected with no regressions. The
showSumGraphchange was separately checked against the original, regressed, and fixed logic across tiny/negative/zero/null sums.🤖 Generated with Claude Code