Merge branch 'ee-merge-request-widget-source-branch-improvements' into 'master'

EE port of merge-request-widget-source-branch-improvements

See merge request gitlab-org/gitlab-ee!4899

app/assets/javascripts/vue_merge_request_widget/components/source_branch_removal_status.vue

+<script>
+  import tooltip from '../../vue_shared/directives/tooltip';
+  import { __ } from '../../locale';
+
+  export default {
+    directives: {
+      tooltip,
+    },
+    created() {
+      this.removesBranchText = __('<strong>Removes</strong> source branch');
+      this.tooltipTitle = __('A user with write access to the source branch selected this option');
+    },
+  };
+</script>
+
+<template>
+  <p
+    v-once
+    class="mr-info-list mr-links source-branch-removal-status append-bottom-0"
+  >
+    <span
+      class="status-text"
+      v-html="removesBranchText"
+    >
+    </span>
+    <i
+      v-tooltip
+      class="fa fa-question-circle"
+      :title="tooltipTitle"
+      :aria-label="tooltipTitle"
+    >
+    </i>
+  </p>
+</template>

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js

@@ -94,7 +94,7 @@ export default {
         || this.mr.preventMerge);
     },
     isRemoveSourceBranchButtonDisabled() {
-      return this.isMergeButtonDisabled || !this.mr.canRemoveSourceBranch;
+      return this.isMergeButtonDisabled;
     },
     shouldShowSquashBeforeMerge() {
       const { commitsCount, enableSquashBeforeMerge } = this.mr;
@@ -286,7 +286,7 @@ export default {
           </span>
           <div class="media-body-wrap space-children">
             <template v-if="shouldShowMergeControls()">
-              <label>
+              <label v-if="mr.canRemoveSourceBranch">
                 <input
                   id="remove-source-branch-input"
                   v-model="removeSourceBranch"

app/assets/javascripts/vue_merge_request_widget/dependencies.js

@@ -40,7 +40,9 @@ export { default as MRWidgetStore } from 'ee/vue_merge_request_widget/stores/mr_
 export { default as MRWidgetService } from 'ee/vue_merge_request_widget/services/mr_widget_service';
 export { default as eventHub } from './event_hub';
 export { default as getStateKey } from 'ee/vue_merge_request_widget/stores/get_state_key';
-export { default as mrWidgetOptions } from 'ee/vue_merge_request_widget/mr_widget_options';
 export { default as stateMaps } from 'ee/vue_merge_request_widget/stores/state_maps';
 export { default as SquashBeforeMerge } from 'ee/vue_merge_request_widget/components/states/mr_widget_squash_before_merge';
 export { default as notify } from '../lib/utils/notify';
+export { default as SourceBranchRemovalStatus } from './components/source_branch_removal_status.vue';
+
+export { default as mrWidgetOptions } from 'ee/vue_merge_request_widget/mr_widget_options';

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js

@@ -33,6 +33,7 @@ import {
   stateMaps,
   SquashBeforeMerge,
   notify,
+  SourceBranchRemovalStatus,
 } from './dependencies';
 import { setFavicon } from '../lib/utils/common_utils';
 
@@ -69,6 +70,9 @@ export default {
     shouldRenderDeployments() {
       return this.mr.deployments.length;
     },
+    shouldRenderSourceBranchRemovalStatus() {
+      return !this.mr.canRemoveSourceBranch && this.mr.shouldRemoveSourceBranch;
+    },
   },
   methods: {
     createService(store) {
@@ -235,6 +239,7 @@ export default {
     'mr-widget-merge-when-pipeline-succeeds': MergeWhenPipelineSucceedsState,
     'mr-widget-auto-merge-failed': AutoMergeFailed,
     'mr-widget-rebase': RebaseState,
+    SourceBranchRemovalStatus,
   },
   template: `
     <div class="mr-state-widget prepend-top-default">
@@ -260,6 +265,9 @@ export default {
           v-if="shouldRenderRelatedLinks"
           :state="mr.state"
           :related-links="mr.relatedLinks" />
+        <source-branch-removal-status
+          v-if="shouldRenderSourceBranchRemovalStatus"
+        />
       </div>
       <div
         class="mr-widget-footer"

changelogs/unreleased/merge-request-widget-source-branch-improvements.yml

+---
+title: Fixes remove source branch checkbox being visible when user cannot remove the
+  branch
+merge_request:
+author:
+type: changed

ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js

@@ -373,12 +373,15 @@ export default {
           :is="componentName"
           :mr="mr"
           :service="service" />
-        <mr-widget-related-links
-          v-if="shouldRenderRelatedLinks"
-          :related-links="mr.relatedLinks"
-          />
         <mr-widget-maintainer-edit
           :maintainerEditAllowed="mr.maintainerEditAllowed" />
+        <mr-widget-related-links
+          v-if="shouldRenderRelatedLinks"
+          :state="mr.state"
+          :related-links="mr.relatedLinks" />
+        <source-branch-removal-status
+          v-if="shouldRenderSourceBranchRemovalStatus"
+        />
       </div>
       <div class="mr-widget-footer" v-if="shouldRenderMergeHelp">
         <mr-widget-merge-help />

spec/features/merge_request/user_sees_merge_widget_spec.rb

 require 'rails_helper'
 
 describe 'Merge request > User sees merge widget', :js do
+  include ProjectForksHelper
+
   let(:project) { create(:project, :repository) }
   let(:project_only_mwps) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) }
   let(:user) { project.creator }
@@ -285,7 +287,29 @@ describe 'Merge request > User sees merge widget', :js do
     end
 
     it 'user cannot remove source branch' do
-      expect(page).to have_field('remove-source-branch-input', disabled: true)
+      expect(page).not_to have_field('remove-source-branch-input')
+    end
+  end
+
+  context 'user cannot merge project and cannot push to fork', :js do
+    let(:forked_project) { fork_project(project, nil, repository: true) }
+    let(:user2) { create(:user) }
+
+    before do
+      project.add_developer(user2)
+      sign_out(:user)
+      sign_in(user2)
+      merge_request.update(
+        source_project: forked_project,
+        target_project: project,
+        merge_params: { 'force_remove_source_branch' => '1' }
+      )
+      visit project_merge_request_path(project, merge_request)
+    end
+
+    it 'user cannot remove source branch' do
+      expect(page).not_to have_field('remove-source-branch-input')
+      expect(page).to have_content('Removes source branch')
     end
   end
 

spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

@@ -517,13 +517,9 @@ describe('MRWidgetReadyToMerge', () => {
 
   describe('Remove source branch checkbox', () => {
     describe('when user can merge but cannot delete branch', () => {
-      it('isRemoveSourceBranchButtonDisabled should be true', () => {
-        expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true);
-      });
-
       it('should be disabled in the rendered output', () => {
         const checkboxElement = vm.$el.querySelector('#remove-source-branch-input');
-        expect(checkboxElement.getAttribute('disabled')).toBe('disabled');
+        expect(checkboxElement).toBeNull();
       });
     });
 
@@ -540,7 +536,7 @@ describe('MRWidgetReadyToMerge', () => {
 
       it('should be enabled in rendered output', () => {
         const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input');
-        expect(checkboxElement.getAttribute('disabled')).toBeNull();
+        expect(checkboxElement).not.toBeNull();
       });
     });
   });
@@ -549,12 +545,12 @@ describe('MRWidgetReadyToMerge', () => {
     describe('when allowed to merge', () => {
       beforeEach(() => {
         vm = createComponent({
-          mr: { isMergeAllowed: true },
+          mr: { isMergeAllowed: true, canRemoveSourceBranch: true },
         });
       });
 
       it('shows remove source branch checkbox', () => {
-        expect(vm.$el.querySelector('.js-remove-source-branch-checkbox')).toBeDefined();
+        expect(vm.$el.querySelector('.js-remove-source-branch-checkbox')).not.toBeNull();
       });
 
       it('shows modify commit message button', () => {

spec/javascripts/vue_mr_widget/ee_mr_widget_options_spec.js

@@ -781,4 +781,28 @@ describe('ee merge request widget options', () => {
       });
     });
   });
+
+  describe('rendering source branch removal status', () => {
+    it('renders when user cannot remove branch and branch should be removed', (done) => {
+      vm = mountComponent(Component, {
+        mrData: {
+          ...mockData,
+        },
+      });
+
+      vm.mr.canRemoveSourceBranch = false;
+      vm.mr.shouldRemoveSourceBranch = true;
+
+      vm.$nextTick(() => {
+        const tooltip = vm.$el.querySelector('.fa-question-circle');
+
+        expect(vm.$el.textContent).toContain('Removes source branch');
+        expect(tooltip.getAttribute('data-original-title')).toBe(
+          'A user with write access to the source branch selected this option',
+        );
+
+        done();
+      });
+    });
+  });
 });

spec/javascripts/vue_mr_widget/mr_widget_options_spec.js

@@ -81,6 +81,29 @@ describe('mrWidgetOptions', () => {
       });
     });
 
+    describe('shouldRenderSourceBranchRemovalStatus', () => {
+      it('should return true when cannot remove source branch and branch will be removed', () => {
+        vm.mr.canRemoveSourceBranch = false;
+        vm.mr.shouldRemoveSourceBranch = true;
+
+        expect(vm.shouldRenderSourceBranchRemovalStatus).toEqual(true);
+      });
+
+      it('should return false when can remove source branch and branch will be removed', () => {
+        vm.mr.canRemoveSourceBranch = true;
+        vm.mr.shouldRemoveSourceBranch = true;
+
+        expect(vm.shouldRenderSourceBranchRemovalStatus).toEqual(false);
+      });
+
+      it('should return false when cannot remove source branch and branch will not be removed', () => {
+        vm.mr.canRemoveSourceBranch = false;
+        vm.mr.shouldRemoveSourceBranch = false;
+
+        expect(vm.shouldRenderSourceBranchRemovalStatus).toEqual(false);
+      });
+    });
+
     describe('shouldRenderDeployments', () => {
       it('should return false for the initial data', () => {
         expect(vm.shouldRenderDeployments).toBeFalsy();
@@ -379,4 +402,22 @@ describe('mrWidgetOptions', () => {
       });
     });
   });
+
+  describe('rendering source branch removal status', () => {
+    it('renders when user cannot remove branch and branch should be removed', (done) => {
+      vm.mr.canRemoveSourceBranch = false;
+      vm.mr.shouldRemoveSourceBranch = true;
+
+      vm.$nextTick(() => {
+        const tooltip = vm.$el.querySelector('.fa-question-circle');
+
+        expect(vm.$el.textContent).toContain('Removes source branch');
+        expect(tooltip.getAttribute('data-original-title')).toBe(
+          'A user with write access to the source branch selected this option',
+        );
+
+        done();
+      });
+    });
+  });
 });