This is an automated email from the ASF dual-hosted git repository.
orangeCatDeveloper pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
The following commit(s) were added to refs/heads/master by this push:
new 8104d7057c [bugfix] validate visual alert rules before saving (#4405)
8104d7057c is described below
commit 8104d7057c72f9c18dea31d32e076750d2c34020
Author: Liming Deng <[email protected]>
AuthorDate: Mon Sep 28 06:11:22 2026 +0800
[bugfix] validate visual alert rules before saving (#4405)
Co-authored-by: Duansg <[email protected]>
---
.../alert-setting/alert-setting.component.spec.ts | 153 +++++++++++++++++++++
.../alert/alert-setting/alert-setting.component.ts | 15 +-
2 files changed, 164 insertions(+), 4 deletions(-)
diff --git
a/web-app/src/app/routes/alert/alert-setting/alert-setting.component.spec.ts
b/web-app/src/app/routes/alert/alert-setting/alert-setting.component.spec.ts
index 5875360bd6..44fdc58cad 100644
--- a/web-app/src/app/routes/alert/alert-setting/alert-setting.component.spec.ts
+++ b/web-app/src/app/routes/alert/alert-setting/alert-setting.component.spec.ts
@@ -18,8 +18,11 @@
*/
import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { FormBuilder, FormControl, NgForm, Validators } from '@angular/forms';
import { configureShallowTest } from '@testing';
+import { NEVER } from 'rxjs';
+import { AlertDefineService } from '../../../service/alert-define.service';
import { AlertSettingComponent } from './alert-setting.component';
describe('AlertSettingComponent', () => {
@@ -40,3 +43,153 @@ describe('AlertSettingComponent', () => {
expect(component).toBeTruthy();
});
});
+
+describe('AlertSettingComponent visual rule validation', () => {
+ let component: AlertSettingComponent;
+ let alertDefineSvc: jasmine.SpyObj<AlertDefineService>;
+
+ beforeEach(() => {
+ alertDefineSvc =
jasmine.createSpyObj<AlertDefineService>('AlertDefineService',
['newAlertDefine', 'editAlertDefine']);
+ alertDefineSvc.newAlertDefine.and.returnValue(NEVER);
+ alertDefineSvc.editAlertDefine.and.returnValue(NEVER);
+ const i18n = jasmine.createSpyObj('I18NService', ['fanyi']);
+ i18n.fanyi.and.callFake((key: string) => key);
+ component = new AlertSettingComponent(
+ jasmine.createSpyObj('NzModalService', ['create']),
+ jasmine.createSpyObj('NzNotificationService', ['success', 'error']),
+ jasmine.createSpyObj('AppDefineService', ['getAppHierarchy']),
+ jasmine.createSpyObj('MonitorService', ['getMonitorsByApp']),
+ alertDefineSvc,
+ i18n,
+ new FormBuilder(),
+ jasmine.createSpyObj('NzMessageService', ['success'])
+ );
+ component.defineForm = new NgForm([], []);
+ component.defineForm.form.addControl('name', new FormControl('test alert',
Validators.required));
+ component.cascadeValues = ['linux', 'cpu'];
+ });
+
+ for (const type of ['realtime_metric', 'realtime_log']) {
+ for (const isAdd of [true, false]) {
+ const action = isAdd ? 'create' : 'update';
+
+ it(`should block ${action} of ${type} with no visual rules`, () => {
+ component.define.type = type;
+ component.isManageModalAdd = isAdd;
+ component.cascadeValues = type === 'realtime_metric' ? ['linux',
'cpu'] : [];
+ component.resetQbDataDefault();
+ expect(component.defineForm.valid).toBeTrue();
+
+ component.onManageModalOk();
+
+ expect(alertDefineSvc.newAlertDefine).not.toHaveBeenCalled();
+ expect(alertDefineSvc.editAlertDefine).not.toHaveBeenCalled();
+ expect(component.isManageModalOkLoading).toBeFalse();
+ expect(component.qbFormCtrl.dirty).toBeTrue();
+ expect(component.qbFormCtrl.touched).toBeTrue();
+ expect(component.qbFormCtrl.hasError('required')).toBeTrue();
+ });
+
+ it(`should block ${action} of ${type} with only empty nested groups`, ()
=> {
+ component.define.type = type;
+ component.isManageModalAdd = isAdd;
+ component.resetQbData({
+ condition: 'and',
+ rules: [{ condition: 'or', rules: [{ condition: 'and', rules: [] }]
}]
+ });
+
+ component.onManageModalOk();
+
+ expect(alertDefineSvc.newAlertDefine).not.toHaveBeenCalled();
+ expect(alertDefineSvc.editAlertDefine).not.toHaveBeenCalled();
+ expect(component.qbFormCtrl.hasError('required')).toBeTrue();
+ });
+
+ it(`should allow ${action} of ${type} with a nested visual condition`,
() => {
+ component.define.type = type;
+ component.isManageModalAdd = isAdd;
+ component.resetQbData({
+ condition: 'and',
+ rules: [{ condition: 'or', rules: [{ field: 'usage', operator: '>',
value: 0 }] }]
+ });
+
+ component.onManageModalOk();
+
+ const save = isAdd ? alertDefineSvc.newAlertDefine :
alertDefineSvc.editAlertDefine;
+ expect(save).toHaveBeenCalledOnceWith(component.define);
+ expect(component.define.expr).toContain('usage > 0');
+ });
+ }
+ }
+
+ it('should allow an existence condition without a comparison value', () => {
+ component.resetQbData({ condition: 'and', rules: [{ field: 'usage',
operator: 'exists' }] });
+
+ component.onManageModalOk();
+
+
expect(alertDefineSvc.newAlertDefine).toHaveBeenCalledOnceWith(component.define);
+ expect(component.define.expr).toContain('exists(usage)');
+ });
+
+ it('should still respect other query-builder validators', () => {
+ component.resetQbData({ condition: 'and', rules: [{ field: 'usage',
operator: '>', value: 90 }] });
+ component.qbFormCtrl.addValidators(() => ({ invalidField: true }));
+
+ component.onManageModalOk();
+
+ expect(alertDefineSvc.newAlertDefine).not.toHaveBeenCalled();
+ });
+
+ for (const type of ['realtime_metric', 'realtime_log']) {
+ it(`should allow switching from invalid visual rules to a valid ${type}
expression`, () => {
+ component.define.type = type;
+ component.onManageModalOk();
+ expect(alertDefineSvc.newAlertDefine).not.toHaveBeenCalled();
+ component.isExpr = true;
+ component.userExpr = 'usage > 90';
+ if (type === 'realtime_log') {
+ component.updateLogFinalExpr();
+ } else {
+ component.updateFinalExpr();
+ }
+
+ component.onManageModalOk();
+
+
expect(alertDefineSvc.newAlertDefine).toHaveBeenCalledOnceWith(component.define);
+ expect(component.defineForm.controls['ruleset']).toBeUndefined();
+ });
+ }
+
+ it('should allow switching from invalid visual rules to availability', () =>
{
+ component.onManageModalOk();
+ expect(alertDefineSvc.newAlertDefine).not.toHaveBeenCalled();
+ component.cascadeValues = ['linux', 'availability'];
+ component.updateFinalExpr();
+
+ component.onManageModalOk();
+
+
expect(alertDefineSvc.newAlertDefine).toHaveBeenCalledOnceWith(component.define);
+ expect(component.define.expr).toContain('equals(__available__,"down")');
+ });
+
+ for (const type of ['periodic_metric', 'periodic_log']) {
+ it(`should not require visual rules for ${type}`, () => {
+ component.define.type = type;
+ component.define.expr = 'periodic query';
+
+ component.onManageModalOk();
+
+
expect(alertDefineSvc.newAlertDefine).toHaveBeenCalledOnceWith(component.define);
+ });
+ }
+
+ it('should retain validation of other required form fields', () => {
+ component.cascadeValues = ['linux', 'availability'];
+ component.defineForm.controls['name'].setValue('');
+
+ component.onManageModalOk();
+
+ expect(alertDefineSvc.newAlertDefine).not.toHaveBeenCalled();
+ expect(component.defineForm.controls['name'].dirty).toBeTrue();
+ });
+});
diff --git
a/web-app/src/app/routes/alert/alert-setting/alert-setting.component.ts
b/web-app/src/app/routes/alert/alert-setting/alert-setting.component.ts
index ef3770377e..81caf6dc3f 100644
--- a/web-app/src/app/routes/alert/alert-setting/alert-setting.component.ts
+++ b/web-app/src/app/routes/alert/alert-setting/alert-setting.component.ts
@@ -99,7 +99,8 @@ export class AlertSettingComponent implements OnInit {
rules: []
};
qbValidator = (control: AbstractControl): ValidationErrors | null => {
- if (!control.value || !control.value.rules || control.value.rules.length
=== 0) {
+ // Empty nested groups must not count as a threshold condition.
+ if (!control.value || !this.ruleset2expr(control.value)) {
return { required: true };
}
return null;
@@ -1100,10 +1101,16 @@ export class AlertSettingComponent implements OnInit {
}
onManageModalOk() {
- if (this.cascadeValues.length == 3) {
- this.defineForm.form.addControl('ruleset', this.qbFormCtrl);
+ const usesVisualRules =
+ !this.isExpr &&
+ (this.define.type === 'realtime_log' ||
+ (this.define.type === 'realtime_metric' && this.cascadeValues.length
=== 2 && this.cascadeValues[1] !== AVAILABILITY));
+ if (usesVisualRules) {
+ this.qbFormCtrl.markAsDirty();
+ this.qbFormCtrl.markAsTouched();
+ this.qbFormCtrl.updateValueAndValidity();
}
- if (this.defineForm?.invalid) {
+ if ((usesVisualRules && this.qbFormCtrl.invalid) ||
this.defineForm?.invalid) {
Object.values(this.defineForm.controls).forEach(control => {
if (control.invalid) {
control.markAsDirty();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
