From cc9a8089f81dfe6136c98e7fce8bd7b0e55fb1f3 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Fri, 14 Jun 2024 07:39:02 +0800 Subject: [PATCH 01/17] Create tests --- .../form.validationRules.tests.js | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) 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 index 808ce09db216..540c25c9a14d 100644 --- 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,62 @@ 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'} validation group should validate without errors (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('validation group should not be removed after its validators are removed (T1233487)', function(assert) { + const $formContainer = $('#form').dxForm({ + validationGroup: 'Test', + formData: { + firstName: 'Kyle', + }, + items: [{ + itemType: 'group', + items: [{ + dataField: 'firstName', + validationRules: [{ type: 'required' }] + }], + }], + }); + const form = $formContainer.dxForm('instance'); + + let $validator = $formContainer.find(`.${VALIDATOR_CLASS}`); + const validator = $validator.dxValidator('instance'); + + assert.equal(validator.option('validationGroup'), 'Test', 'validation group of the validator'); + assert.equal($validator.length, 1, 'validators count'); + + form.option('items[0].items[0]', 'validationRules', []); + $validator = $formContainer.find(`.${VALIDATOR_CLASS}`); + + assert.equal(validator.option('validationGroup'), 'Test', 'validation group should not be removed'); + assert.equal($validator.length, 0, 'validators count'); +}); + QUnit.test('Validate the form without validation rules for an any simple items', function(assert) { const errorStub = sinon.stub(); logger.error = errorStub; From 46fcee7f5b36c6dcd402cde9f71d46e097665f2c Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Fri, 14 Jun 2024 08:23:06 +0800 Subject: [PATCH 02/17] Apply fix --- .../devextreme/js/__internal/ui/form/m_form.ts | 4 ++-- .../js/__internal/ui/m_validation_engine.ts | 14 ++++++++------ .../js/__internal/ui/m_validation_group.ts | 2 +- .../js/__internal/ui/m_validation_summary.ts | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/form/m_form.ts b/packages/devextreme/js/__internal/ui/form/m_form.ts index 574e91aae1fe..64e1f0f5463e 100644 --- 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 index 679ee89d2195..7a9ff9efcac1 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -344,6 +344,7 @@ const GroupConfig = Class.inherit({ ctor(group) { this.group = group; this.validators = []; + this.isRemovable = true; this._pendingValidators = []; this._onValidatorStatusChanged = this._onValidatorStatusChanged.bind(this); this._resetValidationInfo(); @@ -562,10 +563,11 @@ const ValidationEngine = { this.addGroup(); }, - addGroup(group) { + addGroup(group, removable) { let config = this.getGroupConfig(group); if (!config) { config = new GroupConfig(group); + config.isRemovable = removable; this.groups.push(config); } return config; @@ -778,14 +780,13 @@ const ValidationEngine = { }, registerValidatorInGroup(group, validator) { - const groupConfig = ValidationEngine.addGroup(group); + const groupConfig = ValidationEngine.addGroup(group, true); groupConfig.registerValidator.call(groupConfig, validator); }, - _shouldRemoveGroup(group, validatorsInGroup) { + _shouldRemoveGroup(group, validatorsInGroup, isGroupRemovable) { const isDefaultGroup = group === undefined; - const isValidationGroupInstance = group && group.NAME === 'dxValidationGroup'; - return !isDefaultGroup && !isValidationGroupInstance && !validatorsInGroup.length; + return !isDefaultGroup && !validatorsInGroup.length && isGroupRemovable; }, removeRegisteredValidator(group, validator) { @@ -793,7 +794,8 @@ const ValidationEngine = { if (config) { config.removeRegisteredValidator.call(config, validator); const validatorsInGroup = config.validators; - if (this._shouldRemoveGroup(group, validatorsInGroup)) { + const isGroupRemovable = config.isRemovable; + if (this._shouldRemoveGroup(group, validatorsInGroup, isGroupRemovable)) { 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 index 9bd94e40c99f..a34b78830761 100644 --- 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, true); } _initMarkup() { diff --git a/packages/devextreme/js/__internal/ui/m_validation_summary.ts b/packages/devextreme/js/__internal/ui/m_validation_summary.ts index 13c7a2284305..1a112152a53f 100644 --- 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(); From 6ccbe9098d213b2e5c8db6aa00c16e74e419f186 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Fri, 14 Jun 2024 10:28:35 +0800 Subject: [PATCH 03/17] Refactor variable names --- .../js/__internal/ui/m_validation_engine.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index 7a9ff9efcac1..e4ab3babf6c2 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -563,11 +563,11 @@ const ValidationEngine = { this.addGroup(); }, - addGroup(group, removable) { + addGroup(group, isRemovable) { let config = this.getGroupConfig(group); if (!config) { config = new GroupConfig(group); - config.isRemovable = removable; + config.isRemovable = isRemovable; this.groups.push(config); } return config; @@ -784,18 +784,17 @@ const ValidationEngine = { groupConfig.registerValidator.call(groupConfig, validator); }, - _shouldRemoveGroup(group, validatorsInGroup, isGroupRemovable) { + _shouldRemoveGroup(group, validators, isRemovable) { const isDefaultGroup = group === undefined; - return !isDefaultGroup && !validatorsInGroup.length && isGroupRemovable; + return !isDefaultGroup && !validators.length && isRemovable; }, removeRegisteredValidator(group, validator) { const config = ValidationEngine.getGroupConfig(group); if (config) { config.removeRegisteredValidator.call(config, validator); - const validatorsInGroup = config.validators; - const isGroupRemovable = config.isRemovable; - if (this._shouldRemoveGroup(group, validatorsInGroup, isGroupRemovable)) { + const { validators, isRemovable } = config; + if (this._shouldRemoveGroup(group, validators, isRemovable)) { this.removeGroup(group); } } From b1eaa78bccd99a4035594217a7df5d8ef73b7673 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Fri, 14 Jun 2024 18:21:24 +0800 Subject: [PATCH 04/17] refactor fix --- packages/devextreme/js/__internal/ui/form/m_form.ts | 4 ++-- packages/devextreme/js/__internal/ui/m_validation_engine.ts | 6 +++--- packages/devextreme/js/__internal/ui/m_validation_group.ts | 2 +- .../devextreme/js/__internal/ui/m_validation_summary.ts | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/form/m_form.ts b/packages/devextreme/js/__internal/ui/form/m_form.ts index 64e1f0f5463e..b281278cae14 100644 --- 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(), false); + ValidationEngine.addGroup(this._getValidationGroup(), true); 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(), false); + ValidationEngine.addGroup(this._getValidationGroup(), true); 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 index e4ab3babf6c2..e66bf602583a 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -344,7 +344,7 @@ const GroupConfig = Class.inherit({ ctor(group) { this.group = group; this.validators = []; - this.isRemovable = true; + this.isRemovable = false; this._pendingValidators = []; this._onValidatorStatusChanged = this._onValidatorStatusChanged.bind(this); this._resetValidationInfo(); @@ -780,13 +780,13 @@ const ValidationEngine = { }, registerValidatorInGroup(group, validator) { - const groupConfig = ValidationEngine.addGroup(group, true); + const groupConfig = ValidationEngine.addGroup(group, false); groupConfig.registerValidator.call(groupConfig, validator); }, _shouldRemoveGroup(group, validators, isRemovable) { const isDefaultGroup = group === undefined; - return !isDefaultGroup && !validators.length && isRemovable; + return !isDefaultGroup && !validators.length && !isRemovable; }, removeRegisteredValidator(group, validator) { diff --git a/packages/devextreme/js/__internal/ui/m_validation_group.ts b/packages/devextreme/js/__internal/ui/m_validation_group.ts index a34b78830761..5f6fa9f85127 100644 --- 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, true); + 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 index 1a112152a53f..49a826bf9b8e 100644 --- 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, true); + const groupConfig = ValidationEngine.addGroup(group, false); this._unsubscribeGroup(); From f8e66aaf1e46ace7d774494d1b930d32c43b8b69 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Tue, 18 Jun 2024 22:31:39 +0800 Subject: [PATCH 05/17] remove unnecessary test --- .../form.validationRules.tests.js | 29 ------------------- 1 file changed, 29 deletions(-) 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 index 540c25c9a14d..3c8bf3ea6be9 100644 --- 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 @@ -502,35 +502,6 @@ QUnit.test('Changing an validationRules options of an any item does not invalida }); }); -QUnit.test('validation group should not be removed after its validators are removed (T1233487)', function(assert) { - const $formContainer = $('#form').dxForm({ - validationGroup: 'Test', - formData: { - firstName: 'Kyle', - }, - items: [{ - itemType: 'group', - items: [{ - dataField: 'firstName', - validationRules: [{ type: 'required' }] - }], - }], - }); - const form = $formContainer.dxForm('instance'); - - let $validator = $formContainer.find(`.${VALIDATOR_CLASS}`); - const validator = $validator.dxValidator('instance'); - - assert.equal(validator.option('validationGroup'), 'Test', 'validation group of the validator'); - assert.equal($validator.length, 1, 'validators count'); - - form.option('items[0].items[0]', 'validationRules', []); - $validator = $formContainer.find(`.${VALIDATOR_CLASS}`); - - assert.equal(validator.option('validationGroup'), 'Test', 'validation group should not be removed'); - assert.equal($validator.length, 0, 'validators count'); -}); - QUnit.test('Validate the form without validation rules for an any simple items', function(assert) { const errorStub = sinon.stub(); logger.error = errorStub; From 9cf2b5dbbeeebb0ab7f8c03aa544c69a576b74ea Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Tue, 18 Jun 2024 22:47:43 +0800 Subject: [PATCH 06/17] fix logic and fix broken encapsulation --- .../js/__internal/ui/form/m_form.ts | 4 ++-- .../js/__internal/ui/m_validation_engine.ts | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/form/m_form.ts b/packages/devextreme/js/__internal/ui/form/m_form.ts index b281278cae14..64e1f0f5463e 100644 --- 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(), true); + 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(), true); + 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 index e66bf602583a..aedcb617634b 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -341,10 +341,10 @@ const rulesValidators = { }; const GroupConfig = Class.inherit({ - ctor(group) { + ctor(group, isRemovable) { this.group = group; this.validators = []; - this.isRemovable = false; + this._isRemovable = isRemovable; this._pendingValidators = []; this._onValidatorStatusChanged = this._onValidatorStatusChanged.bind(this); this._resetValidationInfo(); @@ -566,8 +566,7 @@ const ValidationEngine = { addGroup(group, isRemovable) { let config = this.getGroupConfig(group); if (!config) { - config = new GroupConfig(group); - config.isRemovable = isRemovable; + config = new GroupConfig(group, isRemovable); this.groups.push(config); } return config; @@ -780,21 +779,23 @@ const ValidationEngine = { }, registerValidatorInGroup(group, validator) { - const groupConfig = ValidationEngine.addGroup(group, false); + const groupConfig = ValidationEngine.addGroup(group, true); groupConfig.registerValidator.call(groupConfig, validator); }, - _shouldRemoveGroup(group, validators, isRemovable) { + _shouldRemoveGroup(group, validatorsInGroup, isRemovable) { const isDefaultGroup = group === undefined; - return !isDefaultGroup && !validators.length && !isRemovable; + const isValidationGroupInstance = group && group.NAME === 'dxValidationGroup'; + return !isDefaultGroup && !isValidationGroupInstance && !validatorsInGroup.length && isRemovable; }, removeRegisteredValidator(group, validator) { const config = ValidationEngine.getGroupConfig(group); if (config) { config.removeRegisteredValidator.call(config, validator); - const { validators, isRemovable } = config; - if (this._shouldRemoveGroup(group, validators, isRemovable)) { + const validatorsInGroup = config.validators; + const isRemovable = config._isRemovable; + if (this._shouldRemoveGroup(group, validatorsInGroup, isRemovable)) { this.removeGroup(group); } } From c8749860b9d3db20aa0fb5abd1acd1063392b8c1 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 12:30:30 +0800 Subject: [PATCH 07/17] refactor validationGroup test --- packages/devextreme/js/__internal/ui/m_validation_engine.ts | 3 +-- .../DevExpress.ui.widgets.editors/validationGroup.tests.js | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index aedcb617634b..9aee48f67ec3 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -785,8 +785,7 @@ const ValidationEngine = { _shouldRemoveGroup(group, validatorsInGroup, isRemovable) { const isDefaultGroup = group === undefined; - const isValidationGroupInstance = group && group.NAME === 'dxValidationGroup'; - return !isDefaultGroup && !isValidationGroupInstance && !validatorsInGroup.length && isRemovable; + return !isDefaultGroup && !validatorsInGroup.length && isRemovable; }, removeRegisteredValidator(group, validator) { 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 index a214d0b74d93..7e2b2578092e 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/validationGroup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/validationGroup.tests.js @@ -268,7 +268,7 @@ QUnit.module('General', { ValidationEngine.removeRegisteredValidator(group, validator2); }); - QUnit.test('group should be validated positively after all validators remove (T1006667)', function(assert) { + QUnit.test('group should be validated positively and should not be removed after all validators are removed (T1006667)', function(assert) { const $container = $('#dxValidationGroup'); const group = this.fixture.createGroup($container); const adapter = sinon.createStubInstance(DefaultAdapter); @@ -293,7 +293,6 @@ QUnit.module('General', { }); const validator2 = $validator2.dxValidator('instance'); - $validator1.appendTo($container); $validator2.appendTo($container); triggerShownEvent($container); @@ -305,6 +304,8 @@ QUnit.module('General', { assert.ok(isValid, 'validation is correct'); }); ValidationEngine.removeRegisteredValidator(group, validator2); + + assert.ok(ValidationEngine.getGroupConfig(group), 'group is not removed'); }); QUnit.test('group should be validated positively with a new validator (async)', function(assert) { From bcf7ab24f7d0f52b0117df135f481cd21f72b335 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 13:01:33 +0800 Subject: [PATCH 08/17] skip special check for default group --- .../devextreme/js/__internal/ui/m_validation_engine.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index 9aee48f67ec3..f7f97cab117a 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -783,9 +783,9 @@ const ValidationEngine = { groupConfig.registerValidator.call(groupConfig, validator); }, - _shouldRemoveGroup(group, validatorsInGroup, isRemovable) { - const isDefaultGroup = group === undefined; - return !isDefaultGroup && !validatorsInGroup.length && isRemovable; + _shouldRemoveGroup(validatorsInGroup, isRemovable) { + debugger + return !validatorsInGroup.length && isRemovable; }, removeRegisteredValidator(group, validator) { @@ -794,7 +794,7 @@ const ValidationEngine = { config.removeRegisteredValidator.call(config, validator); const validatorsInGroup = config.validators; const isRemovable = config._isRemovable; - if (this._shouldRemoveGroup(group, validatorsInGroup, isRemovable)) { + if (this._shouldRemoveGroup(validatorsInGroup, isRemovable)) { this.removeGroup(group); } } From 5a2883fa56ff15b64b0c24d79f84b0228c075876 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 13:02:54 +0800 Subject: [PATCH 09/17] remove debugger --- packages/devextreme/js/__internal/ui/m_validation_engine.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index f7f97cab117a..eb4abcd88d9c 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -784,7 +784,6 @@ const ValidationEngine = { }, _shouldRemoveGroup(validatorsInGroup, isRemovable) { - debugger return !validatorsInGroup.length && isRemovable; }, From 830b39d99808ae11c2786a081a69c3d3d05517a4 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 22:21:21 +0800 Subject: [PATCH 10/17] refactor shouldRemoveGroup function --- .../devextreme/js/__internal/ui/m_validation_engine.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index eb4abcd88d9c..8f80813e45b6 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -783,17 +783,15 @@ const ValidationEngine = { groupConfig.registerValidator.call(groupConfig, validator); }, - _shouldRemoveGroup(validatorsInGroup, isRemovable) { - return !validatorsInGroup.length && isRemovable; - }, - removeRegisteredValidator(group, validator) { const config = ValidationEngine.getGroupConfig(group); if (config) { config.removeRegisteredValidator.call(config, validator); const validatorsInGroup = config.validators; const isRemovable = config._isRemovable; - if (this._shouldRemoveGroup(validatorsInGroup, isRemovable)) { + + const shouldRemoveGroup = validatorsInGroup.length === 0 && isRemovable; + if (shouldRemoveGroup) { this.removeGroup(group); } } From daf73a4cac0e007e5fc9a5f13c3cb1498f71d603 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 22:23:30 +0800 Subject: [PATCH 11/17] rename form test --- .../DevExpress.ui.widgets.form/form.validationRules.tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 3c8bf3ea6be9..c0b19904715d 100644 --- 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 @@ -476,7 +476,7 @@ QUnit.test('Changing an validationRules options of an any item does not invalida }); [false, true].forEach(hasValidationGroup => { - QUnit.test(`form ${hasValidationGroup ? 'with' : 'without'} validation group should validate without errors (T1233487)`, function(assert) { + 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: { From 913f41339e3295625f6fa15bc9fb8a0f1abcdb35 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 22:29:28 +0800 Subject: [PATCH 12/17] add default parameter value --- packages/devextreme/js/__internal/ui/m_validation_engine.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index 8f80813e45b6..6d23da7423c1 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -563,7 +563,7 @@ const ValidationEngine = { this.addGroup(); }, - addGroup(group, isRemovable) { + addGroup(group, isRemovable = true) { let config = this.getGroupConfig(group); if (!config) { config = new GroupConfig(group, isRemovable); @@ -779,7 +779,7 @@ const ValidationEngine = { }, registerValidatorInGroup(group, validator) { - const groupConfig = ValidationEngine.addGroup(group, true); + const groupConfig = ValidationEngine.addGroup(group); groupConfig.registerValidator.call(groupConfig, validator); }, From 9b86a2e8e5b2a547a739e7a9b25da52911d3f557 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 22:39:21 +0800 Subject: [PATCH 13/17] refactor validationGroup tests --- .../validationGroup.tests.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 index 7e2b2578092e..b26ef3c5b0ad 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/validationGroup.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/validationGroup.tests.js @@ -268,7 +268,7 @@ QUnit.module('General', { ValidationEngine.removeRegisteredValidator(group, validator2); }); - QUnit.test('group should be validated positively and should not be removed after all validators are removed (T1006667)', function(assert) { + QUnit.test('group should be validated positively after all validators remove (T1006667)', function(assert) { const $container = $('#dxValidationGroup'); const group = this.fixture.createGroup($container); const adapter = sinon.createStubInstance(DefaultAdapter); @@ -293,6 +293,7 @@ QUnit.module('General', { }); const validator2 = $validator2.dxValidator('instance'); + $validator1.appendTo($container); $validator2.appendTo($container); triggerShownEvent($container); @@ -304,6 +305,18 @@ QUnit.module('General', { assert.ok(isValid, 'validation is correct'); }); 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 = $('
').dxValidator({ + adapter: adapter + }); + const validator1 = $validator1.dxValidator('instance'); + + ValidationEngine.removeRegisteredValidator(group, validator1); assert.ok(ValidationEngine.getGroupConfig(group), 'group is not removed'); }); From a0f62b785c7c0fd9347e6c176282881461d67446 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 22:51:26 +0800 Subject: [PATCH 14/17] fix qunit --- packages/devextreme/js/__internal/ui/m_validation_engine.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index 6d23da7423c1..8f80813e45b6 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -563,7 +563,7 @@ const ValidationEngine = { this.addGroup(); }, - addGroup(group, isRemovable = true) { + addGroup(group, isRemovable) { let config = this.getGroupConfig(group); if (!config) { config = new GroupConfig(group, isRemovable); @@ -779,7 +779,7 @@ const ValidationEngine = { }, registerValidatorInGroup(group, validator) { - const groupConfig = ValidationEngine.addGroup(group); + const groupConfig = ValidationEngine.addGroup(group, true); groupConfig.registerValidator.call(groupConfig, validator); }, From ed5e8cae5a8f976fdac4650f50c05bc8bfe2b995 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 23:28:15 +0800 Subject: [PATCH 15/17] fix validationSummary memory leak --- packages/devextreme/js/__internal/ui/m_validation_summary.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_summary.ts b/packages/devextreme/js/__internal/ui/m_validation_summary.ts index 49a826bf9b8e..1a112152a53f 100644 --- 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, false); + const groupConfig = ValidationEngine.addGroup(group, true); this._unsubscribeGroup(); From dce603761de8298fa7f7c8b136b4c369491a6cc4 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 23:33:16 +0800 Subject: [PATCH 16/17] set default group to non removable --- packages/devextreme/js/__internal/ui/m_validation_engine.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index 8f80813e45b6..73a44e117dca 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -560,7 +560,7 @@ const ValidationEngine = { initGroups() { this.groups = []; - this.addGroup(); + this.addGroup(undefined, false); }, addGroup(group, isRemovable) { From a3d3b51763ff995849707aac971fb31bb597c4d3 Mon Sep 17 00:00:00 2001 From: Joshua Victoria Date: Wed, 19 Jun 2024 23:34:41 +0800 Subject: [PATCH 17/17] add default parameter value --- packages/devextreme/js/__internal/ui/m_validation_engine.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/devextreme/js/__internal/ui/m_validation_engine.ts b/packages/devextreme/js/__internal/ui/m_validation_engine.ts index 73a44e117dca..2e16c4df9e42 100644 --- a/packages/devextreme/js/__internal/ui/m_validation_engine.ts +++ b/packages/devextreme/js/__internal/ui/m_validation_engine.ts @@ -563,7 +563,7 @@ const ValidationEngine = { this.addGroup(undefined, false); }, - addGroup(group, isRemovable) { + addGroup(group, isRemovable = true) { let config = this.getGroupConfig(group); if (!config) { config = new GroupConfig(group, isRemovable); @@ -779,7 +779,7 @@ const ValidationEngine = { }, registerValidatorInGroup(group, validator) { - const groupConfig = ValidationEngine.addGroup(group, true); + const groupConfig = ValidationEngine.addGroup(group); groupConfig.registerValidator.call(groupConfig, validator); },