Prioritize group settings, improve panel titles, disable submit without changes

11 files changed, 160 insertions, 12 deletions

diff --git a/spec/features/groups/group_settings_spec.rb b/spec/features/groups/group_settings_spec.rb
index 08fd9f8af2a..2cdbdcffbc3 100644
--- a/spec/features/groups/group_settings_spec.rb
+++ b/spec/features/groups/group_settings_spec.rb
@@ -125,7 +125,7 @@ describe 'Edit group settings' do
 
   def save_group
     page.within('.gs-general') do
-      click_button 'Save group'
+      click_button 'Save changes'
     end
   end
 end
diff --git a/spec/features/groups/share_lock_spec.rb b/spec/features/groups/share_lock_spec.rb
index 5bbe77019ca..704d9f12888 100644
--- a/spec/features/groups/share_lock_spec.rb
+++ b/spec/features/groups/share_lock_spec.rb
@@ -60,14 +60,14 @@ describe 'Group share with group lock' do
   def enable_group_lock
     page.within('.gs-permissions') do
       check 'group_share_with_group_lock'
-      click_on 'Save group'
+      click_on 'Save changes'
     end
   end
 
   def disable_group_lock
     page.within('.gs-permissions') do
       uncheck 'group_share_with_group_lock'
-      click_on 'Save group'
+      click_on 'Save changes'
     end
   end
 end
diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb
index e62bd6f8187..63aa26cf5fd 100644
--- a/spec/features/groups_spec.rb
+++ b/spec/features/groups_spec.rb
@@ -140,10 +140,13 @@ describe 'Group' do
       visit path
     end
 
+    it_behaves_like 'dirty submit form', [{ form: '.js-general-settings-form', input: 'input[name="group[name]"]' },
+                                          { form: '.js-general-permissions-form', input: 'input[name="group[two_factor_grace_period]"]' }]
+
     it 'saves new settings' do
       page.within('.gs-general') do
         fill_in 'group_name', with: new_name
-        click_button 'Save group'
+        click_button 'Save changes'
       end
 
       expect(page).to have_content 'successfully updated'
diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
index a07edc42eae..72b53bae46a 100644
--- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
+++ b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
@@ -15,7 +15,7 @@ describe 'User uploads avatar to group' do
     )
 
     page.within('.gs-general') do
-      click_button 'Save group'
+      click_button 'Save changes'
     end
 
     visit group_path(group)
diff --git a/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js
new file mode 100644
index 00000000000..87a26183b63
--- /dev/null
+++ b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js
@@ -0,0 +1,25 @@
+import DirtySubmitCollection from '~/dirty_submit/dirty_submit_collection';
+import { setInput, createForm } from './helper';
+
+describe('DirtySubmitCollection', () => {
+  it('disables submits until there are changes', done => {
+    const testElementsCollection = [createForm(), createForm()];
+    const forms = testElementsCollection.map(testElements => testElements.form);
+
+    new DirtySubmitCollection(forms); // eslint-disable-line no-new
+
+    testElementsCollection.forEach(testElements => {
+      const { input, submit } = testElements;
+      const originalValue = input.value;
+
+      expect(submit.disabled).toBe(true);
+
+      return setInput(input, `${originalValue} changes`)
+        .then(() => expect(submit.disabled).toBe(false))
+        .then(() => setInput(input, originalValue))
+        .then(() => expect(submit.disabled).toBe(true))
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+});
diff --git a/spec/javascripts/dirty_submit/dirty_submit_factory_spec.js b/spec/javascripts/dirty_submit/dirty_submit_factory_spec.js
new file mode 100644
index 00000000000..40843a68582
--- /dev/null
+++ b/spec/javascripts/dirty_submit/dirty_submit_factory_spec.js
@@ -0,0 +1,18 @@
+import dirtySubmitFactory from '~/dirty_submit/dirty_submit_factory';
+import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
+import DirtySubmitCollection from '~/dirty_submit/dirty_submit_collection';
+import { createForm } from './helper';
+
+describe('DirtySubmitCollection', () => {
+  it('returns a DirtySubmitForm instance for single form elements', () => {
+    const { form } = createForm();
+
+    expect(dirtySubmitFactory(form) instanceof DirtySubmitForm).toBe(true);
+  });
+
+  it('returns a DirtySubmitCollection instance for a collection of form elements', () => {
+    const forms = [createForm().form, createForm().form];
+
+    expect(dirtySubmitFactory(forms) instanceof DirtySubmitCollection).toBe(true);
+  });
+});
diff --git a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js
new file mode 100644
index 00000000000..86d53fa984a
--- /dev/null
+++ b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js
@@ -0,0 +1,21 @@
+
+import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
+import { setInput, createForm } from './helper';
+
+describe('DirtySubmitForm', () => {
+  it('disables submit until there are changes', done => {
+    const { form, input, submit } = createForm();
+    const originalValue = input.value;
+
+    new DirtySubmitForm(form); // eslint-disable-line no-new
+
+    expect(submit.disabled).toBe(true);
+
+    return setInput(input, `${originalValue} changes`)
+      .then(() => expect(submit.disabled).toBe(false))
+      .then(() => setInput(input, originalValue))
+      .then(() => expect(submit.disabled).toBe(true))
+      .then(done)
+      .catch(done.fail);
+  });
+});
diff --git a/spec/javascripts/dirty_submit/helper.js b/spec/javascripts/dirty_submit/helper.js
new file mode 100644
index 00000000000..6d1e643553c
--- /dev/null
+++ b/spec/javascripts/dirty_submit/helper.js
@@ -0,0 +1,31 @@
+import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
+import setTimeoutPromiseHelper from '../helpers/set_timeout_promise_helper';
+
+export function setInput(element, value) {
+  element.value = value;
+
+  element.dispatchEvent(
+    new Event('input', {
+      bubbles: true,
+      cancelable: true,
+    }),
+  );
+
+  return setTimeoutPromiseHelper(DirtySubmitForm.THROTTLE_DURATION);
+}
+
+export function createForm() {
+  const form = document.createElement('form');
+  form.innerHTML = `
+    <input type="text" value="original" class="js-input" name="input" />
+    <button type="submit" class="js-dirty-submit"></button>
+  `;
+  const input = form.querySelector('.js-input');
+  const submit = form.querySelector('.js-dirty-submit');
+
+  return {
+    form,
+    input,
+    submit,
+  };
+}
diff --git a/spec/javascripts/fixtures/groups.rb b/spec/javascripts/fixtures/groups.rb
index a2035ceae15..b42f442557c 100644
--- a/spec/javascripts/fixtures/groups.rb
+++ b/spec/javascripts/fixtures/groups.rb
@@ -17,6 +17,16 @@ describe 'Groups (JavaScript fixtures)', type: :controller do
     sign_in(admin)
   end
 
+  describe GroupsController, '(JavaScript fixtures)', type: :controller do
+    it 'groups/edit.html.raw' do |example|
+      get :edit,
+        id: group
+
+      expect(response).to be_success
+      store_frontend_fixture(response, example.description)
+    end
+  end
+
   describe Groups::Settings::CiCdController, '(JavaScript fixtures)', type: :controller do
     it 'groups/ci_cd_settings.html.raw' do |example|
       get :show,
diff --git a/spec/javascripts/settings_panels_spec.js b/spec/javascripts/settings_panels_spec.js
index c1a69bd7018..3b681a9ff28 100644
--- a/spec/javascripts/settings_panels_spec.js
+++ b/spec/javascripts/settings_panels_spec.js
@@ -1,10 +1,11 @@
+import $ from 'jquery';
 import initSettingsPanels from '~/settings_panels';
 
 describe('Settings Panels', () => {
-  preloadFixtures('projects/ci_cd_settings.html.raw');
+  preloadFixtures('groups/edit.html.raw');
 
   beforeEach(() => {
-    loadFixtures('projects/ci_cd_settings.html.raw');
+    loadFixtures('groups/edit.html.raw');
   });
 
   describe('initSettingsPane', () => {
@@ -13,17 +14,32 @@ describe('Settings Panels', () => {
     });
 
     it('should expand linked hash fragment panel', () => {
-      window.location.hash = '#autodevops-settings';
+      window.location.hash = '#js-general-settings';
 
-      const pipelineSettingsPanel = document.querySelector('#autodevops-settings');
+      const panel = document.querySelector('#js-general-settings');
       // Our test environment automatically expands everything so we need to clear that out first
-      pipelineSettingsPanel.classList.remove('expanded');
+      panel.classList.remove('expanded');
 
-      expect(pipelineSettingsPanel.classList.contains('expanded')).toBe(false);
+      expect(panel.classList.contains('expanded')).toBe(false);
 
       initSettingsPanels();
 
-      expect(pipelineSettingsPanel.classList.contains('expanded')).toBe(true);
+      expect(panel.classList.contains('expanded')).toBe(true);
     });
   });
+
+  it('does not change the text content of triggers', () => {
+    const panel = document.querySelector('#js-general-settings');
+    const trigger = panel.querySelector('.js-settings-toggle-trigger-only');
+    const originalText = trigger.textContent;
+
+    initSettingsPanels();
+
+    expect(panel.classList.contains('expanded')).toBe(true);
+
+    $(trigger).click();
+
+    expect(panel.classList.contains('expanded')).toBe(false);
+    expect(trigger.textContent).toEqual(originalText);
+  });
 });
diff --git a/spec/support/shared_examples/dirty_submit_form_shared_examples.rb b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb
new file mode 100644
index 00000000000..ba363593120
--- /dev/null
+++ b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb
@@ -0,0 +1,24 @@
+shared_examples 'dirty submit form' do |selector_args|
+  selectors = selector_args.is_a?(Array) ? selector_args : [selector_args]
+
+  selectors.each do |selector|
+    it "disables #{selector[:form]} submit until there are changes", :js do
+      form = find(selector[:form])
+      submit = form.first('.js-dirty-submit')
+      input = form.first(selector[:input])
+      original_value = input.value
+
+      expect(submit.disabled?).to be true
+
+      input.set("#{original_value} changes")
+
+      form.find('.js-dirty-submit:not([disabled])', match: :first)
+      expect(submit.disabled?).to be false
+
+      input.set(original_value)
+
+      form.find('.js-dirty-submit[disabled]', match: :first)
+      expect(submit.disabled?).to be true
+    end
+  end
+end