Frun1na opened a new pull request, #6822:
URL: https://github.com/apache/rocketmq-dashboard/pull/6822
### Which Issue(s) This PR Fixes
- Fixes #6819
### Brief Description
The alert rule editor's condition select rewrote the threshold field
whenever the Unavailable condition was picked
(`web/src/pages/ops/alerts.tsx:1239-1241`: `if (operator === 'UNAVAILABLE')
form.setFieldValue('threshold', 0)`), and nothing restored the value when the
operator was switched back to a comparison. Creating a rule that reads `> 1`
and toggling the condition through Unavailable therefore saved `> 0` — for an
availability metric (0/1) that is a rule which fires on every available sample,
and `< 0` never fires at all. The same zero survived when the metric was
switched to one without the condition, because that path resets the operator by
itself (`:1166-1170`).
The write is also unnecessary: while the operator is Unavailable the
threshold input is not rendered, so the request built by
`form.validateFields()` does not carry a `threshold` key at all — the server
stores 0 for it (`AlertRuleRequestDTO.threshold` is a primitive `double`).
Removing the write keeps the request and the stored value exactly as before,
and preserves the value the user typed. The user-visible change: the threshold
the operator was set to is still in the field when a comparison is picked again.
### Other consumers
Every other surface reads the threshold of an Unavailable rule from the
stored rule (0), not from the form field:
- `web/src/pages/ops/alerts.tsx:803-806` (submit) and `:843-846` (rule test)
— `form.validateFields()` omits the unmounted threshold input, so both payloads
keep sending no `threshold` for an Unavailable rule; unchanged by this PR.
- `web/src/pages/ops/alerts.tsx:258` — the notification template preview
already substitutes `threshold = 0` when the operator is Unavailable; unchanged
and still consistent with what the server stores.
- `web/src/pages/ops/alerts.tsx:679` — `formatThresholdCondition` renders
the "unavailable" label instead of a number for these rules; unchanged.
- `server/.../ops/alert/AlertRuleEvaluator.java:34-37` — native evaluation
short-circuits to the availability check and never compares the threshold;
unchanged.
- `server/.../ops/alert/AlertService.java:687-692` — the Prometheus export
renders `metric == 0` for an Unavailable rule instead of the threshold;
unchanged.
### How Did You Test This Change?
```
$ wb test --filter src/pages/ops/__tests__/AlertsPage.test.tsx
── web (web)
✓ npx eslint src/pages/ops/__tests__/AlertsPage.test.tsx
src/pages/ops/alerts.tsx
✓ npx prettier --check src/pages/ops/__tests__/AlertsPage.test.tsx
src/pages/ops/alerts.tsx
✓ npx vitest run src/pages/ops/__tests__/AlertsPage.test.tsx --maxWorkers=1
--fileParallelism=false
✓ npx tsc -b
Test Files 1 passed (1)
Tests 36 passed (36)
$ wb test # every check the change maps to, no filter
✓ eslint / prettier / vitest related / tsc -b
Test Files 3 passed (3)
Tests 47 passed (47)
```
The new test fails without the fix:
```
FAIL src/pages/ops/__tests__/AlertsPage.test.tsx > AlertsPage > keeps the
threshold the user typed when the operator toggles through Unavailable
Error: expect(element).toHaveValue(1)
Expected the element to have value:
1
Received:
0
❯ src/pages/ops/__tests__/AlertsPage.test.tsx:732:47
Test Files 1 failed (1)
Tests 1 failed | 35 passed (36)
```
The assertion reads the threshold input after the operator goes Unavailable
and back to `>` (`toHaveValue('1')`), and then the payload the dialog submits
(`threshold: 1`). Replacing the removed line puts `0` back into the field, so
both the input and the created rule carry a threshold the user never chose.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`feat:` / `fix:` /
`refactor:` / `chore:` / `docs:` / `perf:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [ ] New UI text has both Chinese and English entries under `web/src/i18n/`
- [ ] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [ ] New source files carry the ASF license header
- [ ] Documentation touched where behaviour changed (README / `docs` /
in-app help)
--
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]
