Form: Fix form validation error when items are cleared (T1233487) (#2…

…7435)
DevExpress · Jun 24, 2024 · e1d72a3 · e1d72a3
1 parent 818419d
commit e1d72a3
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 15 deletions.
diff --git a/packages/devextreme/js/__internal/ui/form/m_form.ts b/packages/devextreme/js/__internal/ui/form/m_form.ts
@@ -264,7 +264,7 @@ const Form = Widget.inherit({
 
   _initMarkup() {
     // @ts-expect-error
-    ValidationEngine.addGroup(this._getValidationGroup());
+    ValidationEngine.addGroup(this._getValidationGroup(), false);
     this._clearCachedInstances();
     this._prepareFormData();
     this.$element().addClass(FORM_CLASS);
@@ -879,7 +879,7 @@ const Form = Widget.inherit({
     const optionName = getOptionNameFromFullName(fullName);
     if (ITEM_OPTIONS_FOR_VALIDATION_UPDATING.includes(optionName)) {
       // @ts-expect-error
-      ValidationEngine.addGroup(this._getValidationGroup());
+      ValidationEngine.addGroup(this._getValidationGroup(), false);
       if (this.option('showValidationSummary')) {
         this._validationSummary?.refreshValidationGroup();
       }

diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts
@@ -341,9 +341,10 @@ const rulesValidators = {
 };
 
 const GroupConfig = Class.inherit({
-  ctor(group) {
+  ctor(group, isRemovable) {
     this.group = group;
     this.validators = [];
+    this._isRemovable = isRemovable;
     this._pendingValidators = [];
     this._onValidatorStatusChanged = this._onValidatorStatusChanged.bind(this);
     this._resetValidationInfo();
@@ -559,13 +560,13 @@ const ValidationEngine = {
 
   initGroups() {
     this.groups = [];
-    this.addGroup();
+    this.addGroup(undefined, false);
   },
 
-  addGroup(group) {
+  addGroup(group, isRemovable = true) {
     let config = this.getGroupConfig(group);
     if (!config) {
-      config = new GroupConfig(group);
+      config = new GroupConfig(group, isRemovable);
       this.groups.push(config);
     }
     return config;
@@ -782,18 +783,15 @@ const ValidationEngine = {
     groupConfig.registerValidator.call(groupConfig, validator);
   },
 
-  _shouldRemoveGroup(group, validatorsInGroup) {
-    const isDefaultGroup = group === undefined;
-    const isValidationGroupInstance = group && group.NAME === 'dxValidationGroup';
-    return !isDefaultGroup && !isValidationGroupInstance && !validatorsInGroup.length;
-  },
-
   removeRegisteredValidator(group, validator) {
     const config = ValidationEngine.getGroupConfig(group);
     if (config) {
       config.removeRegisteredValidator.call(config, validator);
       const validatorsInGroup = config.validators;
-      if (this._shouldRemoveGroup(group, validatorsInGroup)) {
+      const isRemovable = config._isRemovable;
+
+      const shouldRemoveGroup = validatorsInGroup.length === 0 && isRemovable;
+      if (shouldRemoveGroup) {
         this.removeGroup(group);
       }
     }

diff --git a/packages/devextreme/js/__internal/ui/m_validation_group.ts b/packages/devextreme/js/__internal/ui/m_validation_group.ts
@@ -19,7 +19,7 @@ class ValidationGroup extends DOMComponent {
   _init() {
     // @ts-expect-error
     super._init();
-    ValidationEngine.addGroup(this);
+    ValidationEngine.addGroup(this, false);
   }
 
   _initMarkup() {

diff --git a/packages/devextreme/js/__internal/ui/m_validation_summary.ts b/packages/devextreme/js/__internal/ui/m_validation_summary.ts
@@ -37,7 +37,7 @@ const ValidationSummary = CollectionWidget.inherit({
     const $element = this.$element();
     const group = this.option('validationGroup')
             || ValidationEngine.findGroup($element, this._modelByElement($element));
-    const groupConfig = ValidationEngine.addGroup(group);
+    const groupConfig = ValidationEngine.addGroup(group, true);
 
     this._unsubscribeGroup();
 

diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/validationGroup.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/validationGroup.tests.js
@@ -307,6 +307,20 @@ QUnit.module('General', {
         ValidationEngine.removeRegisteredValidator(group, validator2);
     });
 
+    QUnit.test('group should not be removed after its validators are removed (T1233487)', function(assert) {
+        const $container = $('#dxValidationGroup');
+        const group = this.fixture.createGroup($container);
+        const adapter = sinon.createStubInstance(DefaultAdapter);
+        const $validator1 = $('<div>').dxValidator({
+            adapter: adapter
+        });
+        const validator1 = $validator1.dxValidator('instance');
+
+        ValidationEngine.removeRegisteredValidator(group, validator1);
+
+        assert.ok(ValidationEngine.getGroupConfig(group), 'group is not removed');
+    });
+
     QUnit.test('group should be validated positively with a new validator (async)', function(assert) {
         const $container = $('#dxValidationGroup');
         const group = this.fixture.createGroup($container);

diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.form/form.validationRules.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.form/form.validationRules.tests.js
@@ -475,6 +475,33 @@ QUnit.test('Changing an validationRules options of an any item does not invalida
     assert.strictEqual(renderComponentSpy.callCount, 0, 'renderComponentSpy.callCount');
 });
 
+[false, true].forEach(hasValidationGroup => {
+    QUnit.test(`form ${hasValidationGroup ? 'with' : 'without'} validationGroup property specified should validate without errors after nested validators remove (T1233487)`, function(assert) {
+        const form = $('#form').dxForm({
+            validationGroup: hasValidationGroup ? 'test' : undefined,
+            formData: {
+                firstName: 'Kyle',
+            },
+            items: [{
+                itemType: 'group',
+                items: [{
+                    dataField: 'firstName',
+                    validationRules: [{ type: 'required' }]
+                }],
+            }],
+        }).dxForm('instance');
+
+        form.option('items[0].items', []);
+
+        try {
+            form.validate();
+            assert.ok(true);
+        } catch(e) {
+            assert.ok(false, 'error is thrown');
+        }
+    });
+});
+
 QUnit.test('Validate the form without validation rules for an any simple items', function(assert) {
     const errorStub = sinon.stub();
     logger.error = errorStub;