Merge branch '344104-project-feature-gl-toggle-edit' into 'master'

Migrate edit protected branch form to GlToggle

See merge request gitlab-org/gitlab!80549

app/assets/javascripts/protected_branches/protected_branch_edit.js

@@ -3,6 +3,7 @@ import createFlash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
 import { __ } from '~/locale';
 import AccessDropdown from '~/projects/settings/access_dropdown';
+import { initToggle } from '~/toggles';
 import { ACCESS_LEVELS, LEVEL_TYPES } from './constants';
 
 export default class ProtectedBranchEdit {
@@ -14,8 +15,6 @@ export default class ProtectedBranchEdit {
     this.$wrap = options.$wrap;
     this.$allowedToMergeDropdown = this.$wrap.find('.js-allowed-to-merge');
     this.$allowedToPushDropdown = this.$wrap.find('.js-allowed-to-push');
-    this.$forcePushToggle = this.$wrap.find('.js-force-push-toggle');
-    this.$codeOwnerToggle = this.$wrap.find('.js-code-owner-toggle');
 
     this.$wraps[ACCESS_LEVELS.MERGE] = this.$allowedToMergeDropdown.closest(
       `.${ACCESS_LEVELS.MERGE}-container`,
@@ -25,38 +24,45 @@ export default class ProtectedBranchEdit {
     );
 
     this.buildDropdowns();
-    this.bindEvents();
+    this.initToggles();
   }
 
-  bindEvents() {
-    this.$forcePushToggle.on('click', this.onForcePushToggleClick.bind(this));
+  initToggles() {
+    const wrap = this.$wrap.get(0);
+
+    const forcePushToggle = initToggle(wrap.querySelector('.js-force-push-toggle'));
+    forcePushToggle.$on('change', (value) => {
+      forcePushToggle.isLoading = true;
+      forcePushToggle.disabled = true;
+      this.updateProtectedBranch(
+        {
+          allow_force_push: value,
+        },
+        () => {
+          forcePushToggle.isLoading = false;
+          forcePushToggle.disabled = false;
+        },
+      );
+    });
+
     if (this.hasLicense) {
-      this.$codeOwnerToggle.on('click', this.onCodeOwnerToggleClick.bind(this));
+      const codeOwnerToggle = initToggle(wrap.querySelector('.js-code-owner-toggle'));
+      codeOwnerToggle.$on('change', (value) => {
+        codeOwnerToggle.isLoading = true;
+        codeOwnerToggle.disabled = true;
+        this.updateProtectedBranch(
+          {
+            code_owner_approval_required: value,
+          },
+          () => {
+            codeOwnerToggle.isLoading = false;
+            codeOwnerToggle.disabled = false;
+          },
+        );
+      });
     }
   }
 
-  onForcePushToggleClick() {
-    this.$forcePushToggle.toggleClass('is-checked');
-    this.$forcePushToggle.prop('disabled', true);
-
-    const formData = {
-      allow_force_push: this.$forcePushToggle.hasClass('is-checked'),
-    };
-
-    this.updateProtectedBranch(formData, () => this.$forcePushToggle.prop('disabled', false));
-  }
-
-  onCodeOwnerToggleClick() {
-    this.$codeOwnerToggle.toggleClass('is-checked');
-    this.$codeOwnerToggle.prop('disabled', true);
-
-    const formData = {
-      code_owner_approval_required: this.$codeOwnerToggle.hasClass('is-checked'),
-    };
-
-    this.updateProtectedBranch(formData, () => this.$codeOwnerToggle.prop('disabled', false));
-  }
-
   updateProtectedBranch(formData, callback) {
     axios
       .patch(this.$wrap.data('url'), {

app/views/shared/projects/protected_branches/_update_protected_branch.html.haml

@@ -34,4 +34,8 @@
       = _('Members of %{group} can also push to this branch: %{branch}') % { group: (group_push_access_levels.size > 1 ? 'these groups' : 'this group'), branch: group_push_access_levels.map(&:humanize).to_sentence }
 
 %td
-  = render "shared/buttons/project_feature_toggle", is_checked: protected_branch.allow_force_push, label: s_("ProtectedBranch|Toggle allowed to force push"), class_list: "js-force-push-toggle project-feature-toggle", data: { qa_selector: 'force_push_toggle_button', qa_branch_name: protected_branch.name }
+  = render "shared/gl_toggle",
+    classes: 'js-force-push-toggle',
+    label: s_("ProtectedBranch|Toggle allowed to force push"),
+    is_checked: protected_branch.allow_force_push,
+    label_position: 'hidden'

ee/app/views/projects/protected_branches/ee/_code_owner_approval_table.html.haml

 - if @project.feature_available?(:code_owner_approval_required)
   %td
-    = render "shared/buttons/project_feature_toggle", is_checked: protected_branch.code_owner_approval_required, label: s_("ProtectedBranch|Toggle code owner approval"), class_list: "js-code-owner-toggle project-feature-toggle mr-5", data: { qa_selector: 'code_owner_toggle_button', qa_branch_name: protected_branch.name }
+    = render "shared/gl_toggle",
+      classes: 'js-code-owner-toggle gl-mr-5',
+      label: s_("ProtectedBranch|Toggle code owner approval"),
+      is_checked: protected_branch.code_owner_approval_required,
+      label_position: 'hidden',
+      data: { qa_selector: 'code_owner_toggle_button', qa_branch_name: protected_branch.name }

ee/app/views/projects/protected_branches/ee/_protected_branch_access_summary.html.haml

@@ -7,4 +7,8 @@
   = render partial: 'projects/settings/ee/access_level_dropdown', locals: { protected_branch: protected_branch, access_levels: protected_branch.push_access_levels, level_frequencies: access_level_frequencies(protected_branch.push_access_levels), input_basic_name: 'push_access_levels', disabled: !can_unprotect, toggle_class: 'js-allowed-to-push' }
 
 %td
-  = render "shared/buttons/project_feature_toggle", is_checked: protected_branch.allow_force_push, label: s_("ProtectedBranch|Toggle allowed to force push"), class_list: "js-force-push-toggle project-feature-toggle", data: { qa_selector: 'force_push_toggle_button', qa_branch_name: protected_branch.name }
+  = render "shared/gl_toggle",
+    classes: 'js-force-push-toggle',
+    label: s_("ProtectedBranch|Toggle allowed to force push"),
+    is_checked: protected_branch.allow_force_push,
+    label_position: 'hidden'

ee/spec/features/protected_branches_spec.rb

@@ -88,7 +88,7 @@ RSpec.describe 'Protected Branches', :js do
           end
 
           it 'displays toggle on' do
-            expect(page).to have_css('.js-code-owner-toggle.is-checked')
+            expect(page).to have_css('.js-code-owner-toggle button.is-checked')
           end
         end
 
@@ -99,7 +99,7 @@ RSpec.describe 'Protected Branches', :js do
             visit project_settings_repository_path(project)
 
             page.within '.qa-protected-branches-list' do # rubocop:disable QA/SelectorUsage
-              expect(page).not_to have_css('.js-code-owner-toggle.is-checked')
+              expect(page).not_to have_css('.js-code-owner-toggle button.is-checked')
             end
           end
         end

ee/spec/frontend/protected_branches/protected_branch_edit_spec.js

-import MockAdapter from 'axios-mock-adapter';
-import $ from 'jquery';
-import { TEST_HOST } from 'helpers/test_constants';
-import createFlash from '~/flash';
-import axios from '~/lib/utils/axios_utils';
-import ProtectedBranchEdit from '~/protected_branches/protected_branch_edit';
-
-jest.mock('~/flash');
-
-const TEST_URL = `${TEST_HOST}/url`;
-const IS_CHECKED_CLASS = 'is-checked';
-
-describe('EE ProtectedBranchEdit', () => {
-  let mock;
-
-  beforeEach(() => {
-    setFixtures(`<div id="wrap" data-url="${TEST_URL}">
-      <button class="js-code-owner-toggle">Toggle</button>
-    </div>`);
-
-    jest.spyOn(ProtectedBranchEdit.prototype, 'buildDropdowns').mockImplementation();
-
-    mock = new MockAdapter(axios);
-  });
-
-  const findCodeOwnerToggle = () => document.querySelector('.js-code-owner-toggle');
-
-  const create = ({ isChecked = false }) => {
-    if (isChecked) {
-      findCodeOwnerToggle().classList.add(IS_CHECKED_CLASS);
-    }
-
-    return new ProtectedBranchEdit({ $wrap: $('#wrap'), hasLicense: true });
-  };
-
-  afterEach(() => {
-    mock.restore();
-  });
-
-  describe('when unchecked toggle button', () => {
-    let toggle;
-
-    beforeEach(() => {
-      create({ isChecked: false });
-
-      toggle = findCodeOwnerToggle();
-    });
-
-    it('is not changed', () => {
-      expect(toggle).not.toHaveClass(IS_CHECKED_CLASS);
-      expect(toggle).not.toBeDisabled();
-    });
-
-    describe('when clicked', () => {
-      beforeEach(() => {
-        mock
-          .onPatch(TEST_URL, { protected_branch: { code_owner_approval_required: true } })
-          .replyOnce(200, {});
-
-        toggle.click();
-      });
-
-      it('checks and disables button', () => {
-        expect(toggle).toHaveClass(IS_CHECKED_CLASS);
-        expect(toggle).toBeDisabled();
-      });
-
-      it('sends update to BE', () =>
-        axios.waitForAll().then(() => {
-          // Args are asserted in the `.onPatch` call
-          expect(mock.history.patch).toHaveLength(1);
-
-          expect(toggle).not.toBeDisabled();
-          expect(createFlash).not.toHaveBeenCalled();
-        }));
-    });
-
-    describe('when clicked and BE error', () => {
-      beforeEach(() => {
-        mock.onPatch(TEST_URL).replyOnce(500);
-        toggle.click();
-      });
-
-      it('flashes error', () =>
-        axios.waitForAll().then(() => {
-          expect(createFlash).toHaveBeenCalled();
-        }));
-    });
-  });
-});

qa/qa/ee/page/project/settings/protected_branches.rb

@@ -30,7 +30,7 @@ module QA
             end
 
             def require_code_owner_approval(branch)
-              toggle = find_element(:code_owner_toggle_button, branch_name: branch)
+              toggle = find_element(:code_owner_toggle_button, branch_name: branch).find_button('button')
               toggle.click unless toggle[:class].include?('is-checked')
             end
 

spec/frontend/protected_branches/protected_branch_edit_spec.js

@@ -8,59 +8,101 @@ import ProtectedBranchEdit from '~/protected_branches/protected_branch_edit';
 jest.mock('~/flash');
 
 const TEST_URL = `${TEST_HOST}/url`;
+const FORCE_PUSH_TOGGLE_TESTID = 'force-push-toggle';
+const CODE_OWNER_TOGGLE_TESTID = 'code-owner-toggle';
 const IS_CHECKED_CLASS = 'is-checked';
+const IS_DISABLED_CLASS = 'is-disabled';
+const IS_LOADING_SELECTOR = '.toggle-loading';
 
 describe('ProtectedBranchEdit', () => {
   let mock;
 
   beforeEach(() => {
-    setFixtures(`<div id="wrap" data-url="${TEST_URL}">
-      <button class="js-force-push-toggle">Toggle</button>
-    </div>`);
-
     jest.spyOn(ProtectedBranchEdit.prototype, 'buildDropdowns').mockImplementation();
 
     mock = new MockAdapter(axios);
   });
 
-  const findForcePushesToggle = () => document.querySelector('.js-force-push-toggle');
+  const findForcePushToggle = () =>
+    document.querySelector(`div[data-testid="${FORCE_PUSH_TOGGLE_TESTID}"] button`);
+  const findCodeOwnerToggle = () =>
+    document.querySelector(`div[data-testid="${CODE_OWNER_TOGGLE_TESTID}"] button`);
 
-  const create = ({ isChecked = false }) => {
-    if (isChecked) {
-      findForcePushesToggle().classList.add(IS_CHECKED_CLASS);
-    }
+  const create = ({
+    forcePushToggleChecked = false,
+    codeOwnerToggleChecked = false,
+    hasLicense = true,
+  } = {}) => {
+    setFixtures(`<div id="wrap" data-url="${TEST_URL}">
+      <span
+        class="js-force-push-toggle"
+        data-label="Toggle allowed to force push"
+        data-is-checked="${forcePushToggleChecked}"
+        data-testid="${FORCE_PUSH_TOGGLE_TESTID}"></span>
+      <span
+        class="js-code-owner-toggle"
+        data-label="Toggle code owner approval"
+        data-is-checked="${codeOwnerToggleChecked}"
+        data-testid="${CODE_OWNER_TOGGLE_TESTID}"></span>
+    </div>`);
 
-    return new ProtectedBranchEdit({ $wrap: $('#wrap'), hasLicense: false });
+    return new ProtectedBranchEdit({ $wrap: $('#wrap'), hasLicense });
   };
 
   afterEach(() => {
     mock.restore();
   });
 
-  describe('when unchecked toggle button', () => {
+  describe('when license supports code owner approvals', () => {
+    beforeEach(() => {
+      create();
+    });
+
+    it('instantiates the code owner toggle', () => {
+      expect(findCodeOwnerToggle()).not.toBe(null);
+    });
+  });
+
+  describe('when license does not support code owner approvals', () => {
+    beforeEach(() => {
+      create({ hasLicense: false });
+    });
+
+    it('does not instantiate the code owner toggle', () => {
+      expect(findCodeOwnerToggle()).toBe(null);
+    });
+  });
+
+  describe.each`
+    description     | checkedOption               | patchParam                        | finder
+    ${'force push'} | ${'forcePushToggleChecked'} | ${'allow_force_push'}             | ${findForcePushToggle}
+    ${'code owner'} | ${'codeOwnerToggleChecked'} | ${'code_owner_approval_required'} | ${findCodeOwnerToggle}
+  `('when unchecked $description toggle button', ({ checkedOption, patchParam, finder }) => {
     let toggle;
 
     beforeEach(() => {
-      create({ isChecked: false });
+      create({ [checkedOption]: false });
 
-      toggle = findForcePushesToggle();
+      toggle = finder();
     });
 
     it('is not changed', () => {
       expect(toggle).not.toHaveClass(IS_CHECKED_CLASS);
-      expect(toggle).not.toBeDisabled();
+      expect(toggle.querySelector(IS_LOADING_SELECTOR)).toBe(null);
+      expect(toggle).not.toHaveClass(IS_DISABLED_CLASS);
     });
 
     describe('when clicked', () => {
       beforeEach(() => {
-        mock.onPatch(TEST_URL, { protected_branch: { allow_force_push: true } }).replyOnce(200, {});
+        mock.onPatch(TEST_URL, { protected_branch: { [patchParam]: true } }).replyOnce(200, {});
 
         toggle.click();
       });
 
       it('checks and disables button', () => {
         expect(toggle).toHaveClass(IS_CHECKED_CLASS);
-        expect(toggle).toBeDisabled();
+        expect(toggle.querySelector(IS_LOADING_SELECTOR)).not.toBe(null);
+        expect(toggle).toHaveClass(IS_DISABLED_CLASS);
       });
 
       it('sends update to BE', () =>
@@ -68,7 +110,8 @@ describe('ProtectedBranchEdit', () => {
           // Args are asserted in the `.onPatch` call
           expect(mock.history.patch).toHaveLength(1);
 
-          expect(toggle).not.toBeDisabled();
+          expect(toggle).not.toHaveClass(IS_DISABLED_CLASS);
+          expect(toggle.querySelector(IS_LOADING_SELECTOR)).toBe(null);
           expect(createFlash).not.toHaveBeenCalled();
         }));
     });