diff --git a/modules/backend/widgets/form/assets/js/winter.form.js b/modules/backend/widgets/form/assets/js/winter.form.js index e85474603c..56f89a30b7 100644 --- a/modules/backend/widgets/form/assets/js/winter.form.js +++ b/modules/backend/widgets/form/assets/js/winter.form.js @@ -164,13 +164,29 @@ /* * Refresh a dependancy field - * Uses a throttle to prevent duplicate calls and click spamming + * Uses a throttle to prevent duplicate calls and click spamming. + * + * The event parameter is passed automatically by jQuery as the third + * argument (after the two preset arguments from $.proxy). It carries + * a cascadeChain array that tracks which fields have already been + * refreshed in the current cascade, preventing infinite loops when + * fields have circular dependsOn declarations. */ - FormWidget.prototype.onRefreshDependants = function(fieldName, toRefresh) { + FormWidget.prototype.onRefreshDependants = function(fieldName, toRefresh, event) { var self = this, form = this.$el, formEl = this.$form, - fieldElements = this.getFieldElements() + fieldElements = this.getFieldElements(), + cascadeChain = (event && event.cascadeChain) || [] + + /* + * If this field already appears in the cascade chain, we have + * a circular dependency. Stop the cascade to prevent an + * infinite loop. See: https://github.com/wintercms/winter/issues/421 + */ + if (cascadeChain.indexOf(fieldName) !== -1) { + return + } if (this.dependantUpdateTimers[fieldName] !== undefined) { window.clearTimeout(this.dependantUpdateTimers[fieldName]) @@ -186,8 +202,12 @@ data: refreshData }).success(function() { self.toggleEmptyTabs() + + var newChain = cascadeChain.concat([fieldName]) $.each(toRefresh.fields, function(key, field) { - $('[data-field-name="' + field + '"]').trigger('change') + var cascadeEvent = $.Event('change') + cascadeEvent.cascadeChain = newChain + $('[data-field-name="' + field + '"]').trigger(cascadeEvent) }) }) }, this.dependantUpdateInterval) diff --git a/modules/system/tests/js/cases/framework/FormWidgetDependants.test.js b/modules/system/tests/js/cases/framework/FormWidgetDependants.test.js new file mode 100644 index 0000000000..5fa8e1a133 --- /dev/null +++ b/modules/system/tests/js/cases/framework/FormWidgetDependants.test.js @@ -0,0 +1,149 @@ +import FakeDom from '../../helpers/FakeDom'; + +jest.setTimeout(5000); + +describe('Form Widget dependsOn', function () { + /** + * Build a FakeDom with jQuery, WinterCMS foundation, stubs, and the form widget. + * The FormWidgetStubs fixture provides minimal implementations of ocJSON, + * $.fn.render, $.fn.request (synchronous success), and $.fn.loadIndicator. + */ + function buildDom(html) { + return FakeDom + .new() + .addScript([ + 'modules/backend/assets/js/vendor/jquery.min.js', + 'modules/system/assets/ui/js/foundation.baseclass.js', + 'modules/system/assets/ui/js/foundation.controlutils.js', + 'modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js', + 'modules/backend/widgets/form/assets/js/winter.form.js', + ]) + .render(html); + } + + /** + * Build form HTML with fields and their dependsOn declarations. + * + * Collects all field names (both keys and referenced dependencies) and creates + * a div for each. Fields that have dependencies get data-field-depends attributes. + * + * @param {Object} fields - Map of field names to arrays of field names they depend on. + * e.g. { a: ['b'], b: ['a'] } for circular deps. + */ + function buildFormHtml(fields) { + // Collect all unique field names (both dependents and their dependencies) + var allFields = {}; + for (var name in fields) { + allFields[name] = fields[name]; + fields[name].forEach(function (dep) { + if (!(dep in allFields)) { + allFields[dep] = null; + } + }); + } + + var fieldHtml = ''; + for (var fieldName in allFields) { + fieldHtml += '
'; + } + + return '
' + + '
' + + fieldHtml + + '
' + + '
'; + } + + it('refreshes dependent fields when a field changes', function (done) { + buildDom(buildFormHtml({ fieldB: ['fieldA'] })) + .then(function (dom) { + var $ = dom.window.jQuery; + var requestSpy = jest.spyOn($.fn, 'request'); + + $('[data-field-name="fieldA"]').trigger('change'); + + // The form widget debounces with a 300ms timer + setTimeout(function () { + try { + expect(requestSpy).toHaveBeenCalledTimes(1); + expect(requestSpy).toHaveBeenCalledWith( + 'onRefreshField', + expect.objectContaining({ + data: expect.objectContaining({ fields: ['fieldB'] }) + }) + ); + done(); + } catch (e) { + done(e); + } + }, 500); + }); + }); + + it('prevents infinite loop with circular dependsOn declarations', function (done) { + buildDom(buildFormHtml({ fieldA: ['fieldB'], fieldB: ['fieldA'] })) + .then(function (dom) { + var $ = dom.window.jQuery; + var requestSpy = jest.spyOn($.fn, 'request'); + + $('[data-field-name="fieldA"]').trigger('change'); + + // With 300ms debounce per step, an unguarded circular loop would fire + // many times in 2 seconds. The fix should limit this to exactly 2 + // requests: A refreshes B, B refreshes A (blocked by cascade chain). + setTimeout(function () { + try { + expect(requestSpy.mock.calls.length).toBe(2); + done(); + } catch (e) { + done(e); + } + }, 2000); + }); + }); + + it('allows transitive cascading (A -> B -> C) without blocking', function (done) { + buildDom(buildFormHtml({ fieldB: ['fieldA'], fieldC: ['fieldB'] })) + .then(function (dom) { + var $ = dom.window.jQuery; + var requestSpy = jest.spyOn($.fn, 'request'); + + $('[data-field-name="fieldA"]').trigger('change'); + + // A->B (300ms) then B->C (600ms). Allow time for both. + setTimeout(function () { + try { + expect(requestSpy.mock.calls.length).toBe(2); + done(); + } catch (e) { + done(e); + } + }, 1500); + }); + }); + + it('stops cycle in a three-field circular chain (A -> B -> C -> A)', function (done) { + buildDom(buildFormHtml({ fieldA: ['fieldC'], fieldB: ['fieldA'], fieldC: ['fieldB'] })) + .then(function (dom) { + var $ = dom.window.jQuery; + var requestSpy = jest.spyOn($.fn, 'request'); + + $('[data-field-name="fieldA"]').trigger('change'); + + // A->B (300ms), B->C (600ms), C tries to refresh A but A is already + // in the cascade chain [fieldA, fieldB] so it stops. Total: 3 requests. + setTimeout(function () { + try { + expect(requestSpy.mock.calls.length).toBe(3); + done(); + } catch (e) { + done(e); + } + }, 2000); + }); + }); +}); diff --git a/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js b/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js new file mode 100644 index 0000000000..03f88d04b8 --- /dev/null +++ b/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js @@ -0,0 +1,39 @@ +/* + * Stubs for testing winter.form.js in isolation. + * + * Provides minimal implementations of WinterCMS dependencies that the + * form widget relies on at load time. + */ + +// ocJSON - used by paramToObj inside the form widget +window.ocJSON = function (str) { + return JSON.parse(str); +}; + +// $(document).render() - used by the form widget to auto-initialize on DOM ready +jQuery.fn.render = function (fn) { + fn(); +}; + +/* + * $.fn.request() - stub that mimics WinterCMS's AJAX framework contract. + * + * Returns a resolved jQuery Deferred with .done()/.fail()/.always() and a + * .success() alias (matching the WinterCMS framework.js Request class). + * The deferred resolves immediately since there is no real network I/O. + */ +jQuery.fn.request = function (handler, options) { + var deferred = jQuery.Deferred(); + + deferred.success = function (fn) { + return deferred.done(fn); + }; + + deferred.resolve(); + return deferred; +}; + +// $.fn.loadIndicator() - no-op stub +jQuery.fn.loadIndicator = function () { + return this; +};