iloveeyjafjalla opened a new pull request, #6670:
URL: https://github.com/apache/rocketmq-dashboard/pull/6670
### Which Issue(s) This PR Fixes
- Fixes #6669
### Brief Description
The rules table and the editor scaled a stored ratio threshold with a plain
`* 100`
(`web/src/pages/ops/alerts.tsx:133-134` and the editor seed at `:469`).
Eight of the ninety-nine whole
percents do not round trip in binary floating point - `0.29 * 100` is
`28.999999999999996` - so a legacy
ratio rule (no `thresholdUnit`) printed `> 28.999999999999996%`, and saving
it unchanged wrote
`28.999999999999996` with `thresholdUnit: "%"` into the field the evaluator
compares against.
- `web/src/utils/format.ts` gains `ratioToPercent`, which shifts the decimal
point of the shortest
round-trip decimal representation instead of multiplying. This is the same
operation as
`AlertNotificationTemplate.formattedValue`'s
`BigDecimal.movePointRight(2)` and keeps its precision:
`0.29` becomes `29`, `0.1234` becomes `12.34`, `0.295` becomes `29.5` - no
one-decimal cap.
- `formatThresholdCondition` and the editor seed both go through the helper.
- No new UI text and no new files, so the i18n and license-header rules are
untouched.
- Boundary: rules already saved through the old dialog keep their stored
residue. This change fixes
what is rendered and written from now on; nothing here rewrites stored
data.
The two earlier attempts on this conversion (#5071, #5485) were closed
without a code change; the
review comment on #5485 asked for exactly this - match the backend's decimal
rendering - and this PR
does that without a new test file, so the frontend build stays green.
### How Did You Test This Change?
Red first, with the added tests in place and only the source change reverted
(`git stash push -- web/src/pages/ops/alerts.tsx web/src/utils/format.ts`):
```
$ cd web && npx vitest run src/pages/ops/__tests__/AlertsPage.test.tsx
--maxWorkers=1
FAIL src/pages/ops/__tests__/AlertsPage.test.tsx > renders legacy native
ratio thresholds as percentages
AssertionError: expected '> 28.999999999999996%' to be '> 29%'
FAIL src/pages/ops/__tests__/AlertsPage.test.tsx > opens a legacy ratio
rule with a clean percent and saves that value back
Received: ... "threshold": 28.999999999999996, "thresholdUnit": "%"
Test Files 1 failed (1)
Tests 3 failed | 31 passed (34)
```
(The third red failure is an unrelated 20 s timeout in `stays on the page
after a bulk delete clears a full page`, which passes in the green run.)
Green with the change:
```
$ npx vitest run src/pages/ops/__tests__/AlertsPage.test.tsx --maxWorkers=1
Test Files 1 passed (1)
Tests 34 passed (34)
$ npx vitest run src/utils/format.test.ts --maxWorkers=1
Test Files 1 passed (1)
Tests 29 passed (29)
$ npx vitest run src/pages/ops --maxWorkers=1
Test Files 12 passed (12)
Tests 116 passed (116)
$ npx tsc -b (clean)
$ npx eslint src/utils/format.ts src/utils/format.test.ts
src/pages/ops/alerts.tsx src/pages/ops/__tests__/AlertsPage.test.tsx
5 problems (0 errors, 5 warnings) - all pre-existing
$ npx prettier --check (same four files)
All matched files use Prettier code style!
```
The two added cases:
- `renders legacy native ratio thresholds as percentages` (extended, kept
the existing `0.85` control)
now also asserts `0.29`, `0.07`, `0.58` and the non-integer `0.1234` - the
values that exposed the
residue.
- `opens a legacy ratio rule with a clean percent and saves that value back`
opens the editor for a
stored `0.29` rule and asserts the save payload carries `threshold: 29,
thresholdUnit: "%"`, not
the residue the form used to hold.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix(alerts): ...`)
- [x] Tests added or updated for non-trivial changes (existing test files
extended, no new file)
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
- no new text
- [x] Architecture constraints stay green - no backend change
- [x] New source files carry the ASF license header - no new files
- [x] Documentation touched where behaviour changed - not applicable
One open PR touches the same file: #5891 translates the stored threshold
unit in the same function's
second branch and adds its cases to the same test file. Whichever of the two
merges second needs a
small rebase; the two changes are otherwise independent.
AI assistance: the analysis, the change and these tests were produced with
AI assistance (an agent
working in this repository) and reviewed by me before submission; the
verification output above is
from the actual runs.
--
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]
