Merge branch '52278-squash-checkbox-fix' into 'master'

Resolve "When merging an MR, the squash checkbox isn't always supported"

Closes #52278

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

6 files changed, 185 insertions, 41 deletions

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
index 84c8a3464a5..5df891aebf3 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
@@ -12,7 +12,7 @@ export default {
   name: 'ReadyToMerge',
   components: {
     statusIcon,
-    'squash-before-merge': SquashBeforeMerge,
+    SquashBeforeMerge,
   },
   props: {
     mr: { type: Object, required: true },
@@ -28,6 +28,7 @@ export default {
       isMakingRequest: false,
       isMergingImmediately: false,
       commitMessage: this.mr.commitMessage,
+      squashBeforeMerge: this.mr.squash,
       successSvg,
       warningSvg,
     };
@@ -110,12 +111,6 @@ export default {
       return enableSquashBeforeMerge && commitsCount > 1;
     },
   },
-  created() {
-    eventHub.$on('MRWidgetUpdateSquash', this.handleUpdateSquash);
-  },
-  beforeDestroy() {
-    eventHub.$off('MRWidgetUpdateSquash', this.handleUpdateSquash);
-  },
   methods: {
     shouldShowMergeControls() {
       return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
@@ -143,7 +138,7 @@ export default {
         commit_message: this.commitMessage,
         merge_when_pipeline_succeeds: this.setToMergeWhenPipelineSucceeds,
         should_remove_source_branch: this.removeSourceBranch === true,
-        squash: this.mr.squash,
+        squash: this.squashBeforeMerge,
       };
 
       this.isMakingRequest = true;
@@ -166,9 +161,6 @@ export default {
           new Flash('Something went wrong. Please try again.'); // eslint-disable-line
         });
     },
-    handleUpdateSquash(val) {
-      this.mr.squash = val;
-    },
     initiateMergePolling() {
       simplePoll((continuePolling, stopPolling) => {
         this.handleMergePolling(continuePolling, stopPolling);
@@ -311,8 +303,9 @@ export default {
             <!-- Placeholder for EE extension of this component -->
             <squash-before-merge
               v-if="shouldShowSquashBeforeMerge"
-              :mr="mr"
-              :is-merge-button-disabled="isMergeButtonDisabled"
+              v-model="squashBeforeMerge"
+              :help-path="mr.squashBeforeMergeHelpPath"
+              :is-disabled="isMergeButtonDisabled"
             />
 
             <span v-if="mr.ffOnlyEnabled" class="js-fast-forward-message">
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/squash_before_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/squash_before_merge.vue
index e71acf0d7dd..bbb9491d6cf 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/squash_before_merge.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/squash_before_merge.vue
@@ -1,6 +1,5 @@
 <script>
 import Icon from '~/vue_shared/components/icon.vue';
-import eventHub from '~/vue_merge_request_widget/event_hub';
 import tooltip from '~/vue_shared/directives/tooltip';
 
 export default {
@@ -11,23 +10,19 @@ export default {
     tooltip,
   },
   props: {
-    mr: {
-      type: Object,
-      required: true,
-    },
-    isMergeButtonDisabled: {
+    value: {
       type: Boolean,
       required: true,
     },
-  },
-  data() {
-    return {
-      squashBeforeMerge: this.mr.squash,
-    };
-  },
-  methods: {
-    updateSquashModel() {
-      eventHub.$emit('MRWidgetUpdateSquash', this.squashBeforeMerge);
+    helpPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    isDisabled: {
+      type: Boolean,
+      required: false,
+      default: false,
     },
   },
 };
@@ -37,18 +32,19 @@ export default {
   <div class="accept-control inline">
     <label class="merge-param-checkbox">
       <input
-        v-model="squashBeforeMerge"
-        :disabled="isMergeButtonDisabled"
+        :checked="value"
+        :disabled="isDisabled"
         type="checkbox"
         name="squash"
         class="qa-squash-checkbox"
-        @change="updateSquashModel"
+        @change="$emit('input', $event.target.checked);"
       />
       {{ __('Squash commits') }}
     </label>
     <a
+      v-if="helpPath"
       v-tooltip
-      :href="mr.squashBeforeMergeHelpPath"
+      :href="helpPath"
       data-title="About this feature"
       data-placement="bottom"
       target="_blank"
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
index b7f12076958..5a9d86594b1 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
@@ -32,7 +32,6 @@ import MRWidgetStore from './stores/ee_switch_mr_widget_store';
 import MRWidgetService from './services/ee_switch_mr_widget_service';
 import eventHub from './event_hub';
 import stateMaps from './stores/ee_switch_state_maps';
-import SquashBeforeMerge from './components/states/squash_before_merge.vue';
 import notify from '~/lib/utils/notify';
 import SourceBranchRemovalStatus from './components/source_branch_removal_status.vue';
 import GroupedTestReportsApp from '../reports/components/grouped_test_reports_app.vue';
@@ -59,7 +58,6 @@ export default {
     'mr-widget-missing-branch': MissingBranchState,
     'mr-widget-ready-to-merge': ReadyToMergeState,
     'sha-mismatch': ShaMismatchState,
-    'mr-widget-squash-before-merge': SquashBeforeMerge,
     'mr-widget-checking': CheckingState,
     'mr-widget-unresolved-discussions': UnresolvedDiscussionsState,
     'mr-widget-pipeline-blocked': PipelineBlockedState,
diff --git a/changelogs/unreleased/52278-squash-checkbox-fix.yml b/changelogs/unreleased/52278-squash-checkbox-fix.yml
new file mode 100644
index 00000000000..c81748ae419
--- /dev/null
+++ b/changelogs/unreleased/52278-squash-checkbox-fix.yml
@@ -0,0 +1,5 @@
+---
+title: Resolve When merging an MR, the squash checkbox isnt always supported
+merge_request: 24296
+author:
+type: fixed
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
index 2119a3b927a..e387367d1a2 100644
--- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
@@ -1,11 +1,12 @@
 import Vue from 'vue';
 import ReadyToMerge from '~/vue_merge_request_widget/components/states/ready_to_merge.vue';
+import SquashBeforeMerge from '~/vue_merge_request_widget/components/states/squash_before_merge.vue';
 import eventHub from '~/vue_merge_request_widget/event_hub';
+import { createLocalVue, shallowMount } from '@vue/test-utils';
 
 const commitMessage = 'This is the commit message';
 const commitMessageWithDescription = 'This is the commit message description';
-const createComponent = (customConfig = {}) => {
-  const Component = Vue.extend(ReadyToMerge);
+const createTestMr = customConfig => {
   const mr = {
     isPipelineActive: false,
     pipeline: null,
@@ -16,6 +17,7 @@ const createComponent = (customConfig = {}) => {
     hasCI: false,
     ciStatus: null,
     sha: '12345678',
+    squash: false,
     commitMessage,
     commitMessageWithDescription,
     shouldRemoveSourceBranch: true,
@@ -24,14 +26,23 @@ const createComponent = (customConfig = {}) => {
 
   Object.assign(mr, customConfig.mr);
 
-  const service = {
-    merge() {},
-    poll() {},
-  };
+  return mr;
+};
+
+const createTestService = () => ({
+  merge() {},
+  poll() {},
+});
+
+const createComponent = (customConfig = {}) => {
+  const Component = Vue.extend(ReadyToMerge);
 
   return new Component({
     el: document.createElement('div'),
-    propsData: { mr, service },
+    propsData: {
+      mr: createTestMr(customConfig),
+      service: createTestService(),
+    },
   });
 };
 
@@ -612,6 +623,47 @@ describe('ReadyToMerge', () => {
     });
   });
 
+  describe('Squash checkbox component', () => {
+    let wrapper;
+    const localVue = createLocalVue();
+
+    const createLocalComponent = (customConfig = {}) => {
+      wrapper = shallowMount(localVue.extend(ReadyToMerge), {
+        localVue,
+        propsData: {
+          mr: createTestMr(customConfig),
+          service: createTestService(),
+        },
+      });
+    };
+
+    afterEach(() => {
+      wrapper.destroy();
+    });
+
+    const findCheckboxElement = () => wrapper.find(SquashBeforeMerge);
+
+    it('should be rendered when squash before merge is enabled and there is more than 1 commit', () => {
+      createLocalComponent({
+        mr: { commitsCount: 2, enableSquashBeforeMerge: true },
+      });
+
+      expect(findCheckboxElement().exists()).toBeTruthy();
+    });
+
+    it('should not be rendered when squash before merge is disabled', () => {
+      createLocalComponent({ mr: { commitsCount: 2, enableSquashBeforeMerge: false } });
+
+      expect(findCheckboxElement().exists()).toBeFalsy();
+    });
+
+    it('should not be rendered when there is only 1 commit', () => {
+      createLocalComponent({ mr: { commitsCount: 1, enableSquashBeforeMerge: true } });
+
+      expect(findCheckboxElement().exists()).toBeFalsy();
+    });
+  });
+
   describe('Merge controls', () => {
     describe('when allowed to merge', () => {
       beforeEach(() => {
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_squash_before_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_squash_before_merge_spec.js
new file mode 100644
index 00000000000..d6d8eecfcb9
--- /dev/null
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_squash_before_merge_spec.js
@@ -0,0 +1,100 @@
+import { createLocalVue, shallowMount } from '@vue/test-utils';
+import SquashBeforeMerge from '~/vue_merge_request_widget/components/states/squash_before_merge.vue';
+
+const localVue = createLocalVue();
+
+describe('Squash before merge component', () => {
+  let wrapper;
+
+  const createComponent = props => {
+    wrapper = shallowMount(localVue.extend(SquashBeforeMerge), {
+      localVue,
+      sync: false,
+      propsData: {
+        ...props,
+      },
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('checkbox', () => {
+    const findCheckbox = () => wrapper.find('.qa-squash-checkbox');
+
+    it('is unchecked if passed value prop is false', () => {
+      createComponent({
+        value: false,
+      });
+
+      expect(findCheckbox().element.checked).toBeFalsy();
+    });
+
+    it('is checked if passed value prop is true', () => {
+      createComponent({
+        value: true,
+      });
+
+      expect(findCheckbox().element.checked).toBeTruthy();
+    });
+
+    it('changes value on click', done => {
+      createComponent({
+        value: false,
+      });
+
+      findCheckbox().element.checked = true;
+
+      findCheckbox().trigger('change');
+
+      wrapper.vm.$nextTick(() => {
+        expect(findCheckbox().element.checked).toBeTruthy();
+        done();
+      });
+    });
+
+    it('is disabled if isDisabled prop is true', () => {
+      createComponent({
+        value: false,
+        isDisabled: true,
+      });
+
+      expect(findCheckbox().attributes('disabled')).toBeTruthy();
+    });
+  });
+
+  describe('about link', () => {
+    it('is not rendered if no help path is passed', () => {
+      createComponent({
+        value: false,
+      });
+
+      const aboutLink = wrapper.find('a');
+
+      expect(aboutLink.exists()).toBeFalsy();
+    });
+
+    it('is rendered if  help path is passed', () => {
+      createComponent({
+        value: false,
+        helpPath: 'test-path',
+      });
+
+      const aboutLink = wrapper.find('a');
+
+      expect(aboutLink.exists()).toBeTruthy();
+    });
+
+    it('should have a correct help path if passed', () => {
+      createComponent({
+        value: false,
+        helpPath: 'test-path',
+      });
+
+      const aboutLink = wrapper.find('a');
+
+      expect(aboutLink.attributes('href')).toEqual('test-path');
+    });
+  });
+});