zmuxuny opened a new pull request, #5546: URL: https://github.com/apache/rocketmq-dashboard/pull/5546
### Which Issue(s) This PR Fixes Fixes #4326. ### Replacement for #4457 This resubmits #4457 after the inactivity workflow automatically closed it unmerged on October 4. The original PR and its review history are retained: - [Maintainer review](https://github.com/apache/rocketmq-dashboard/pull/4457#pullrequestreview-5267292386) - [Response and corrective revision](https://github.com/apache/rocketmq-dashboard/pull/4457#issuecomment-5771213294) - [Automatic closure](https://github.com/apache/rocketmq-dashboard/pull/4457#issuecomment-5975210557) The source branch is `2541c6b06f9a0fab795b90439737d3cb08a51d25`, updated by a non-force merge with `rocketmq-studio@6a68042fa73a513f40436ca721944b4de48181ec`. The reviewed corrective commit `9380d072` remains in its history. This is one three-file backend fix; no frontend, schema or dependency changes are included. ### Problem and state policy Unconditional state deletion on every rule mutation discards a live alert episode and acknowledgement when only presentation or notification settings change. Preserving state for both toggle directions would instead orphan disabled rules, because active evaluation and reconciliation exclude them while the runtime query still exposes their state. The explicit policy is: - Disable: purge evaluation/runtime state and reminder timestamps. - Enable: preserve existing state. - Semantic edit: purge state. - Presentation-only edit of an enabled rule: preserve state. - Delete: continue to purge state. Disabling does not emit a synthetic recovery event. ### Response to the five blocking review points 1. Both toggle overloads and both bulk paths purge when disabling; they no longer leave disabled FIRING rows behind. 2. Stateful regression tests first observe FIRING with notification/reminder timestamps through `listRuleRuntime`, then require the disabled rule's runtime view to be empty. 3. Both edit entrypoints apply the same explicit disable policy even though enabled is absent from the semantic fingerprint. Semantic-change and enable-preserves cases have separate coverage. 4. Reset-on-delete assertions remain intact. The bulk disable-and-delete test retains two purge assertions. Enable-direction expectations intentionally reflect the policy above rather than silently changing the contract. 5. `AlertRuleStateLifecycleTest` covers six disable paths, six enable paths, two semantic edits, two presentation-only edits and four delete paths. The presentation-only case edits an enabled rule. The optional duplicate-read optimization remains separate to keep this revision focused. The maintainer review also distinguished #4391, which changes acknowledgement transaction handling, from this lifecycle fix. ### Validation on the refreshed source On October 4, the published `2541c6b0` commit was fetched and tested locally with Java 21: - `AlertRuleStateLifecycleTest,AlertServiceTest,AlertRuleSemanticFingerprintTest,NativeAlertProcessorTest`: **136/136 passed**, including all **20 lifecycle cases**. - Checkstyle: **0 violations**. - `git diff --check`: passed. - The refreshed source tree was verified identical to the tested prepared tree. The final published commit's focused tests were rerun successfully. Focused command: ```sh cd server mvn -B -ntp -Dtest=AlertRuleStateLifecycleTest,AlertServiceTest,AlertRuleSemanticFingerprintTest,NativeAlertProcessorTest test ``` Local execution used an explicit Mockito Java agent because self-attachment is unavailable. The September 22 red/green experiment in #4457 remains historical evidence: the old implementation failed exactly the six disable cases, and the corrected lifecycle suite passed 20/20. **Build limits:** `mvn -B -ntp -DskipTests package` failed at the backend binary-license gate because packaged dependencies lack complete upstream LICENSE/COPYING material. The historical package success from September 22 is not a current pass. Full external-datasource integration tests were not run locally. No successful CI result is claimed for this new PR; its checks still need to complete. AI assistance was used for implementation, review and validation. ### Checklist - [x] One coherent lifecycle fix - [x] Existing human review addressed explicitly - [x] Regression tests and ASF header on the new test file - [x] Previously reviewed commits retained - [ ] Full CI and backend release-license gate are green -- 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]
