Merge branch '61928-remove-throttle-from-dirty-submit' into 'master'

Make throttling in the dirty_submit form check smarter

Closes #61928

See merge request gitlab-org/gitlab-ce!28377

3 files changed, 97 insertions, 31 deletions

diff --git a/app/assets/javascripts/dirty_submit/dirty_submit_form.js b/app/assets/javascripts/dirty_submit/dirty_submit_form.js
index 765969daa32..0fcaec9531c 100644
--- a/app/assets/javascripts/dirty_submit/dirty_submit_form.js
+++ b/app/assets/javascripts/dirty_submit/dirty_submit_form.js
@@ -21,10 +21,15 @@ class DirtySubmitForm {
   }
 
   registerListeners() {
-    const throttledUpdateDirtyInput = _.throttle(
-      event => this.updateDirtyInput(event),
-      DirtySubmitForm.THROTTLE_DURATION,
+    const getThrottledHandlerForInput = _.memoize(() =>
+      _.throttle(event => this.updateDirtyInput(event), DirtySubmitForm.THROTTLE_DURATION),
     );
+
+    const throttledUpdateDirtyInput = event => {
+      const throttledHandler = getThrottledHandlerForInput(event.target.name);
+      throttledHandler(event);
+    };
+
     this.form.addEventListener('input', throttledUpdateDirtyInput);
     this.form.addEventListener('change', throttledUpdateDirtyInput);
     $(this.form).on('change.select2', throttledUpdateDirtyInput);
diff --git a/changelogs/unreleased/61928-remove-throttle-from-dirty-submit.yml b/changelogs/unreleased/61928-remove-throttle-from-dirty-submit.yml
new file mode 100644
index 00000000000..f8ef5dbb53b
--- /dev/null
+++ b/changelogs/unreleased/61928-remove-throttle-from-dirty-submit.yml
@@ -0,0 +1,6 @@
+---
+title: Fix issue that causes "Save changes" button in project settings pages to be
+  enabled/disabled incorrectly when changes are made to the form
+merge_request: 28377
+author:
+type: fixed
diff --git a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js
index 95cc90dcb0f..b1017e0c4f0 100644
--- a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js
+++ b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js
@@ -1,3 +1,4 @@
+import _ from 'underscore';
 import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
 import { getInputValue, setInputValue, createForm } from './helper';
 
@@ -13,46 +14,100 @@ function expectToToggleDisableOnDirtyUpdate(submit, input) {
 }
 
 describe('DirtySubmitForm', () => {
-  DirtySubmitForm.THROTTLE_DURATION = 0;
+  const originalThrottleDuration = DirtySubmitForm.THROTTLE_DURATION;
 
-  it('disables submit until there are changes', done => {
-    const { form, input, submit } = createForm();
+  describe('submit button tests', () => {
+    beforeEach(() => {
+      DirtySubmitForm.THROTTLE_DURATION = 0;
+    });
 
-    new DirtySubmitForm(form); // eslint-disable-line no-new
+    afterEach(() => {
+      DirtySubmitForm.THROTTLE_DURATION = originalThrottleDuration;
+    });
 
-    return expectToToggleDisableOnDirtyUpdate(submit, input)
-      .then(done)
-      .catch(done.fail);
-  });
+    it('disables submit until there are changes', done => {
+      const { form, input, submit } = createForm();
 
-  it('disables submit until there are changes when initializing with a falsy value', done => {
-    const { form, input, submit } = createForm();
-    input.value = '';
+      new DirtySubmitForm(form); // eslint-disable-line no-new
 
-    new DirtySubmitForm(form); // eslint-disable-line no-new
+      return expectToToggleDisableOnDirtyUpdate(submit, input)
+        .then(done)
+        .catch(done.fail);
+    });
 
-    return expectToToggleDisableOnDirtyUpdate(submit, input)
-      .then(done)
-      .catch(done.fail);
-  });
+    it('disables submit until there are changes when initializing with a falsy value', done => {
+      const { form, input, submit } = createForm();
+      input.value = '';
+
+      new DirtySubmitForm(form); // eslint-disable-line no-new
+
+      return expectToToggleDisableOnDirtyUpdate(submit, input)
+        .then(done)
+        .catch(done.fail);
+    });
 
-  it('disables submit until there are changes for radio inputs', done => {
-    const { form, input, submit } = createForm('radio');
+    it('disables submit until there are changes for radio inputs', done => {
+      const { form, input, submit } = createForm('radio');
 
-    new DirtySubmitForm(form); // eslint-disable-line no-new
+      new DirtySubmitForm(form); // eslint-disable-line no-new
 
-    return expectToToggleDisableOnDirtyUpdate(submit, input)
-      .then(done)
-      .catch(done.fail);
+      return expectToToggleDisableOnDirtyUpdate(submit, input)
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('disables submit until there are changes for checkbox inputs', done => {
+      const { form, input, submit } = createForm('checkbox');
+
+      new DirtySubmitForm(form); // eslint-disable-line no-new
+
+      return expectToToggleDisableOnDirtyUpdate(submit, input)
+        .then(done)
+        .catch(done.fail);
+    });
   });
 
-  it('disables submit until there are changes for checkbox inputs', done => {
-    const { form, input, submit } = createForm('checkbox');
+  describe('throttling tests', () => {
+    beforeEach(() => {
+      jasmine.clock().install();
+      DirtySubmitForm.THROTTLE_DURATION = 100;
+    });
+
+    afterEach(() => {
+      jasmine.clock().uninstall();
+      DirtySubmitForm.THROTTLE_DURATION = originalThrottleDuration;
+    });
+
+    it('throttles updates when rapid changes are made to a single form element', () => {
+      const { form, input } = createForm();
+      const updateDirtyInputSpy = spyOn(new DirtySubmitForm(form), 'updateDirtyInput');
+
+      _.range(10).forEach(i => {
+        setInputValue(input, `change ${i}`, false);
+      });
+
+      jasmine.clock().tick(101);
+
+      expect(updateDirtyInputSpy).toHaveBeenCalledTimes(2);
+    });
+
+    it('does not throttle updates when rapid changes are made to different form elements', () => {
+      const form = document.createElement('form');
+      const range = _.range(10);
+      range.forEach(i => {
+        form.innerHTML += `<input type="text" name="input-${i}" class="js-input-${i}"/>`;
+      });
+
+      const updateDirtyInputSpy = spyOn(new DirtySubmitForm(form), 'updateDirtyInput');
+
+      range.forEach(i => {
+        const input = form.querySelector(`.js-input-${i}`);
+        setInputValue(input, `change`, false);
+      });
 
-    new DirtySubmitForm(form); // eslint-disable-line no-new
+      jasmine.clock().tick(101);
 
-    return expectToToggleDisableOnDirtyUpdate(submit, input)
-      .then(done)
-      .catch(done.fail);
+      expect(updateDirtyInputSpy).toHaveBeenCalledTimes(range.length);
+    });
   });
 });