Frun1na opened a new pull request, #6767:
URL: https://github.com/apache/rocketmq-dashboard/pull/6767
### Which Issue(s) This PR Fixes
- Fixes #6766
### Brief Description
`web/src/services/opsService.ts:484` ended a maintenance window in Mock mode
by rebuilding the store array and returning unconditionally, so an id the store
does not have produced a resolved promise instead of an error:
```ts
export async function deleteAlertSilence(id: number): Promise<void> {
if (!isMockMode()) return opsApi.deleteAlertSilence(id);
alertSilencesState = alertSilencesState.filter((silence) => silence.id !==
id);
}
```
`DELETE /api/alert-silences/{id}` answers 404 for the same input:
`AlertSilenceService.delete` (`AlertSilenceService.java:85`) throws
`BusinessException(404, "Alert silence not found: " + id)` when
`repository.deleteById(id)` is false. `deleteSilence` in
`web/src/pages/ops/systemAlerts.tsx:478` has both branches — it toasts
`sysAlerts.silenceEnded` on resolve and `sysAlerts.silenceEndFailed` on reject
— so the Mock data source reported an ended maintenance window that was never
found, and left the operator with a row that looked closed while the window was
still in force.
The Mock branch now checks the store first and throws the endpoint's
message. That is the shape `deleteAclRule`, `deleteAclUser`, `deleteInstance`,
`deleteK8sCert`, `deleteNameServer` and `acknowledgeAlert` already use in the
same folder.
User-visible change: in Mock mode, ending a maintenance window that no
longer exists now reports `sysAlerts.silenceEndFailed` and keeps the row,
instead of reporting success.
### Other consumers
The same sentinel — a Mock delete that resolves for a record the store does
not have — was enumerated across `web/src/services/` and each surface was
handled:
- `opsService.ts:484` `deleteAlertSilence` — fixed in this PR.
- `opsService.ts:292` `deleteAlertRule` — covered by open PR #6351
(`fix(mock): reject deletes of missing topics, groups and alert rules`); not
touched here.
- `topicService.ts:167` `deleteTopic`, `topicService.ts:182`
`batchDeleteTopics` — covered by #6351.
- `consumerService.ts:354` `deleteConsumerGroup`, `consumerService.ts:467`
`batchDeleteConsumerGroups` — covered by #6351.
- `opsService.ts:408` `acknowledgeAlert` — already correct, throws `System
alert not found: <id>` (#6516).
- `aclService.ts:166` `deleteAclRule`, `aclService.ts:212` `deleteAclUser` —
already correct, both throw.
- `instanceService.ts:162` `deleteInstance` — already correct, throws;
`instanceService.ts:126` `deleteInstancesBatch` already reports per-id failures.
- `clusterService.ts:238` `deleteK8sCert`, `clusterService.ts:387`
`deleteNameServer` — already correct, both throw.
This path is not in scope of #6331, whose `### Scope` reads "Frontend
Mock-mode service contract parity for Topic, Consumer Group, and alert-rule
deletion", nor of the paths it lists as already covered (#920/#921 instances,
#6043 ACL, #1034 backend alert/ACL). It is a distinct endpoint (`DELETE
/api/alert-silences/{id}`) with its own message. There is no hunk overlap with
#6351: that PR edits `deleteAlertRule` around `:295`, this one edits
`deleteAlertSilence` around `:484`.
### How Did You Test This Change?
```
$ ./wb test --filter src/services/opsService.test.ts
✓ 1.4s npx eslint src/services/opsService.test.ts
src/services/opsService.ts
✓ -2.3s npx prettier --check src/services/opsService.test.ts
src/services/opsService.ts
✗ 2.3s npx vitest run src/services/opsService.test.ts --maxWorkers=1
--fileParallelism=false
+ Received:
undefined
❯ src/services/opsService.test.ts:242:47
Test Files 1 failed (1)
Tests 1 failed | 17 passed (18)
$ ./wb test
✓ 1.4s npx eslint src/services/opsService.test.ts
src/services/opsService.ts
✓ -2.3s npx prettier --check src/services/opsService.test.ts
src/services/opsService.ts
✓ 103.4s npx vitest related --run src/services/opsService.test.ts
src/services/opsService.ts
Test Files 8 passed (8)
Tests 121 passed (121)
✓ 4.8s npx tsc -b
```
The new test fails without the fix:
```
# only the five fix lines reverted (git checkout --
web/src/services/opsService.ts), the test kept
$ ./wb test --filter src/services/opsService.test.ts
✗ 2.2s npx vitest run src/services/opsService.test.ts --maxWorkers=1
--fileParallelism=false
+ Received:
undefined
❯ src/services/opsService.test.ts:242:47
Test Files 1 failed (1)
Tests 1 failed | 17 passed (18)
✓ 3.2s npx tsc -b
```
The assertion goes red because it asks `deleteAlertSilence(missingId)` to
reject, and vitest reports `promise resolved "undefined" instead of rejecting`
— an implementation that rebuilds the store array and returns unconditionally
resolves for any id, so the missing-record expectation is exactly what the fix
changes. Its positive control, `still ends a maintenance window the mock
inventory has Test`, passes both with and without the fix, which is what shows
the assertion pins the missing-record case rather than "the function throws".
### 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]
