Merge branch 'nfriend-update-merge-request-widget-for-post-merge-pipelines-ee' into 'master'

Fix merge request pipeline warnings in EE

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

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

+<script>
+import { GlLink } from '@gitlab/ui';
+import Icon from '~/vue_shared/components/icon.vue';
+import { WARNING, DANGER, WARNING_MESSAGE_CLASS, DANGER_MESSAGE_CLASS } from '../constants';
+
+export default {
+  name: 'MrWidgetAlertMessage',
+  components: {
+    GlLink,
+    Icon,
+  },
+  props: {
+    type: {
+      type: String,
+      required: false,
+      default: DANGER,
+      validator: value => [WARNING, DANGER].includes(value),
+    },
+    helpPath: {
+      type: String,
+      required: false,
+      default: undefined,
+    },
+  },
+  computed: {
+    messageClass() {
+      if (this.type === WARNING) {
+        return WARNING_MESSAGE_CLASS;
+      } else if (this.type === DANGER) {
+        return DANGER_MESSAGE_CLASS;
+      }
+
+      return '';
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="m-3 ml-5" :class="messageClass">
+    <slot></slot>
+    <gl-link v-if="helpPath" :href="helpPath" target="_blank">
+      <icon :size="16" name="question-o" class="align-middle" />
+    </gl-link>
+  </div>
+</template>

app/assets/javascripts/vue_merge_request_widget/constants.js

+export const WARNING = 'warning';
+export const DANGER = 'danger';
+
+export const WARNING_MESSAGE_CLASS = 'warning_message';
+export const DANGER_MESSAGE_CLASS = 'danger_message';

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -12,6 +12,7 @@ import WidgetMergeHelp from './components/mr_widget_merge_help.vue';
 import MrWidgetPipelineContainer from './components/mr_widget_pipeline_container.vue';
 import Deployment from './components/deployment.vue';
 import WidgetRelatedLinks from './components/mr_widget_related_links.vue';
+import MrWidgetAlertMessage from './components/mr_widget_alert_message.vue';
 import MergedState from './components/states/mr_widget_merged.vue';
 import ClosedState from './components/states/mr_widget_closed.vue';
 import MergingState from './components/states/mr_widget_merging.vue';
@@ -46,6 +47,7 @@ export default {
     MrWidgetPipelineContainer,
     Deployment,
     'mr-widget-related-links': WidgetRelatedLinks,
+    MrWidgetAlertMessage,
     'mr-widget-merged': MergedState,
     'mr-widget-closed': ClosedState,
     'mr-widget-merging': MergingState,
@@ -110,6 +112,18 @@ export default {
     shouldRenderMergedPipeline() {
       return this.mr.state === 'merged' && !_.isEmpty(this.mr.mergePipeline);
     },
+    showMergePipelineForkWarning() {
+      return Boolean(
+        this.mr.mergePipelinesEnabled && this.mr.sourceProjectId !== this.mr.targetProjectId,
+      );
+    },
+    showTargetBranchAdvancedError() {
+      return Boolean(
+        this.mr.pipeline &&
+          this.mr.pipeline.target_sha &&
+          this.mr.pipeline.target_sha !== this.mr.targetBranchSha,
+      );
+    },
   },
   watch: {
     state(newVal, oldVal) {
@@ -328,6 +342,30 @@ export default {
           :related-links="mr.relatedLinks"
         />
 
+        <mr-widget-alert-message
+          v-if="showMergePipelineForkWarning"
+          type="warning"
+          :help-path="mr.mergeRequestPipelinesHelpPath"
+        >
+          {{
+            s__(
+              'mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result',
+            )
+          }}
+        </mr-widget-alert-message>
+
+        <mr-widget-alert-message
+          v-if="showTargetBranchAdvancedError"
+          type="danger"
+          :help-path="mr.mergeRequestPipelinesHelpPath"
+        >
+          {{
+            s__(
+              'mrWidget|The target branch has advanced, which invalidates the merge request pipeline. Please update the source branch and retry merging',
+            )
+          }}
+        </mr-widget-alert-message>
+
         <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
       </div>
       <div v-if="shouldRenderMergeHelp" class="mr-widget-footer"><mr-widget-merge-help /></div>

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -28,9 +28,11 @@ export default class MergeRequestStore {
     this.iid = data.iid;
     this.title = data.title;
     this.targetBranch = data.target_branch;
+    this.targetBranchSha = data.target_branch_sha;
     this.sourceBranch = data.source_branch;
     this.sourceBranchProtected = data.source_branch_protected;
     this.conflictsDocsPath = data.conflicts_docs_path;
+    this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path;
     this.mergeStatus = data.merge_status;
     this.commitMessage = data.default_merge_commit_message;
     this.shortMergeCommitSha = data.short_merge_commit_sha;
@@ -99,6 +101,9 @@ export default class MergeRequestStore {
     this.allowCollaboration = data.allow_collaboration;
     this.targetProjectFullPath = data.target_project_full_path;
     this.sourceProjectFullPath = data.source_project_full_path;
+    this.sourceProjectId = data.source_project_id;
+    this.targetProjectId = data.target_project_id;
+    this.mergePipelinesEnabled = data.merge_pipelines_enabled;
 
     // Cherry-pick and Revert actions related
     this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;

app/assets/stylesheets/framework/common.scss

@@ -200,12 +200,12 @@ li.note {
   }
 }
 
-.warning_message {
-  border-left: 4px solid $orange-200;
-  color: $orange-700;
+@mixin message($background-color, $border-color, $text-color) {
+  border-left: 4px solid $border-color;
+  color: $text-color;
   padding: 10px;
   margin-bottom: 10px;
-  background: $orange-100;
+  background: $background-color;
   padding-left: 20px;
 
   &.centered {
@@ -213,6 +213,14 @@ li.note {
   }
 }
 
+.warning_message {
+  @include message($orange-100, $orange-200, $orange-700);
+}
+
+.danger_message {
+  @include message($red-100, $red-200, $red-900);
+}
+
 .gitlab-promo {
   a {
     color: $gl-gray-350;

app/presenters/merge_request_presenter.rb

@@ -212,6 +212,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     help_page_path('user/project/merge_requests/resolve_conflicts.md')
   end
 
+  def merge_request_pipelines_docs_path
+    help_page_path('ci/merge_request_pipelines/index.md')
+  end
+
   private
 
   def cached_can_be_reverted?

app/serializers/merge_request_widget_entity.rb

@@ -258,6 +258,10 @@ class MergeRequestWidgetEntity < IssuableEntity
     presenter(merge_request).conflicts_docs_path
   end
 
+  expose :merge_request_pipelines_docs_path do |merge_request|
+    presenter(merge_request).merge_request_pipelines_docs_path
+  end
+
   private
 
   delegate :current_user, to: :request

changelogs/unreleased/nfriend-update-merge-request-widget-for-post-merge-pipelines.yml

+---
+title: Add two new warning messages to the MR widget about merge request pipelines
+merge_request: 25983
+author:
+type: added

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

@@ -298,6 +298,31 @@ export default {
           :state="mr.state"
           :related-links="mr.relatedLinks"
         />
+
+        <mr-widget-alert-message
+          v-if="showMergePipelineForkWarning"
+          type="warning"
+          :help-path="mr.mergeRequestPipelinesHelpPath"
+        >
+          {{
+            s__(
+              'mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result',
+            )
+          }}
+        </mr-widget-alert-message>
+
+        <mr-widget-alert-message
+          v-if="showTargetBranchAdvancedError"
+          type="danger"
+          :help-path="mr.mergeRequestPipelinesHelpPath"
+        >
+          {{
+            s__(
+              'mrWidget|The target branch has advanced, which invalidates the merge request pipeline. Please update the source branch and retry merging',
+            )
+          }}
+        </mr-widget-alert-message>
+
         <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
       </div>
       <div v-if="shouldRenderMergeHelp" class="mr-widget-footer"><mr-widget-merge-help /></div>

ee/spec/features/merge_request/user_sees_merge_widget_spec.rb

+# frozen_string_literal: true
+require 'rails_helper'
+
+describe 'Merge request > User sees merge widget', :js do
+  include ProjectForksHelper
+
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_developer(user)
+    sign_in(user)
+  end
+
+  context 'when merge pipelines option is enabled at project level configuration' do
+    before do
+      stub_licensed_features(merge_pipelines: true)
+      project.update!(merge_pipelines_enabled: true)
+    end
+
+    let!(:merge_request) do
+      create(:merge_request,
+        *traits,
+        source_project: source_project,
+        source_branch: 'feature',
+        target_project: target_project,
+        target_branch: 'master',
+        **options)
+    end
+
+    let(:target_project) { project }
+    let(:source_project) { project }
+
+    context 'when the head pipeline is merge request pipeline' do
+      let(:traits) { [:with_merge_request_pipeline] }
+      let(:options) { {} }
+
+      before do
+        merge_request.update_head_pipeline
+        merge_request.all_pipelines.first.succeed!
+      end
+
+      it 'does not show any warnings' do
+        visit project_merge_request_path(project, merge_request)
+
+        expect(page).not_to have_css('.danger_message')
+      end
+
+      context 'when target branch is advanced' do
+        let(:options) { { target_sha: 'old-commit' } }
+
+        it 'shows a warning that fork project cannot create merge request pipelines' do
+          visit project_merge_request_path(project, merge_request)
+
+          within('.danger_message') do
+            expect(page)
+              .to have_content('The target branch has advanced, which invalidates ' \
+                               'the merge request pipeline. Please update the source ' \
+                               'branch and retry merging')
+          end
+        end
+      end
+    end
+
+    context 'when merge request is submitted from a forked project' do
+      let(:source_project) { fork_project(project, user, repository: true) }
+      let(:traits) { [:with_detached_merge_request_pipeline] }
+      let(:options) { {} }
+
+      it 'shows a warning that fork project cannot create merge request pipelines' do
+        visit project_merge_request_path(project, merge_request)
+
+        within('.warning_message') do
+          expect(page)
+            .to have_content('Fork merge requests do not create merge request' \
+                             ' pipelines which validate a post merge result')
+        end
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -14353,6 +14353,9 @@ msgstr ""
 msgid "mrWidget|Fast-forward merge is not possible. To merge this request, first rebase locally."
 msgstr ""
 
+msgid "mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result"
+msgstr ""
+
 msgid "mrWidget|If the %{branch} branch exists in your local repository, you can merge this merge request manually using the"
 msgstr ""
 
@@ -14477,6 +14480,9 @@ msgstr ""
 msgid "mrWidget|The source branch will not be deleted"
 msgstr ""
 
+msgid "mrWidget|The target branch has advanced, which invalidates the merge request pipeline. Please update the source branch and retry merging"
+msgstr ""
+
 msgid "mrWidget|There are merge conflicts"
 msgstr ""
 

spec/fixtures/api/schemas/entities/merge_request_widget.json

@@ -125,6 +125,7 @@
     "test_reports_path": { "type": ["string", "null"] },
     "can_receive_suggestion": { "type": "boolean" },
     "source_branch_protected": { "type": "boolean" },
-    "conflicts_docs_path": { "type": ["string", "null"] }
+    "conflicts_docs_path": { "type": ["string", "null"] },
+    "merge_request_pipelines_docs_path": { "type": ["string", "null"] }
   }
 }

spec/javascripts/vue_mr_widget/components/mr_widget_alert_message_spec.js

+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import MrWidgetAlertMessage from '~/vue_merge_request_widget/components/mr_widget_alert_message.vue';
+import { GlLink } from '@gitlab/ui';
+
+describe('MrWidgetAlertMessage', () => {
+  let wrapper;
+
+  beforeEach(() => {
+    const localVue = createLocalVue();
+
+    wrapper = shallowMount(localVue.extend(MrWidgetAlertMessage), {
+      propsData: {},
+      localVue,
+    });
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('when type is not provided', () => {
+    it('should render a red message', () => {
+      expect(wrapper.classes()).toContain('danger_message');
+      expect(wrapper.classes()).not.toContain('warning_message');
+    });
+  });
+
+  describe('when type === "danger"', () => {
+    it('should render a red message', () => {
+      wrapper.setProps({ type: 'danger' });
+
+      expect(wrapper.classes()).toContain('danger_message');
+      expect(wrapper.classes()).not.toContain('warning_message');
+    });
+  });
+
+  describe('when type === "warning"', () => {
+    it('should render a red message', () => {
+      wrapper.setProps({ type: 'warning' });
+
+      expect(wrapper.classes()).toContain('warning_message');
+      expect(wrapper.classes()).not.toContain('danger_message');
+    });
+  });
+
+  describe('when helpPath is not provided', () => {
+    it('should not render a help icon/link', () => {
+      const link = wrapper.find(GlLink);
+
+      expect(link.exists()).toBe(false);
+    });
+  });
+
+  describe('when helpPath is provided', () => {
+    it('should render a help icon/link', () => {
+      wrapper.setProps({ helpPath: '/path/to/help/docs' });
+      const link = wrapper.find(GlLink);
+
+      expect(link.exists()).toBe(true);
+      expect(link.attributes().href).toBe('/path/to/help/docs');
+    });
+  });
+});

spec/javascripts/vue_mr_widget/mock_data.js

@@ -243,6 +243,7 @@ export default {
   merge_commit_path:
     'http://localhost:3000/root/acets-app/commit/53027d060246c8f47e4a9310fb332aa52f221775',
   troubleshooting_docs_path: 'help',
+  merge_request_pipelines_docs_path: '/help/ci/merge_request_pipelines/index.md',
   squash: true,
 };
 // Codeclimate

spec/javascripts/vue_mr_widget/mr_widget_options_spec.js

@@ -183,6 +183,85 @@ describe('mrWidgetOptions', () => {
         });
       });
     });
+
+    describe('showMergePipelineForkWarning', () => {
+      describe('when the source project and target project are the same', () => {
+        beforeEach(done => {
+          Vue.set(vm.mr, 'mergePipelinesEnabled', true);
+          Vue.set(vm.mr, 'sourceProjectId', 1);
+          Vue.set(vm.mr, 'targetProjectId', 1);
+          vm.$nextTick(done);
+        });
+
+        it('should be false', () => {
+          expect(vm.showMergePipelineForkWarning).toEqual(false);
+        });
+      });
+
+      describe('when merge pipelines are not enabled', () => {
+        beforeEach(done => {
+          Vue.set(vm.mr, 'mergePipelinesEnabled', false);
+          Vue.set(vm.mr, 'sourceProjectId', 1);
+          Vue.set(vm.mr, 'targetProjectId', 2);
+          vm.$nextTick(done);
+        });
+
+        it('should be false', () => {
+          expect(vm.showMergePipelineForkWarning).toEqual(false);
+        });
+      });
+
+      describe('when merge pipelines are enabled _and_ the source project and target project are different', () => {
+        beforeEach(done => {
+          Vue.set(vm.mr, 'mergePipelinesEnabled', true);
+          Vue.set(vm.mr, 'sourceProjectId', 1);
+          Vue.set(vm.mr, 'targetProjectId', 2);
+          vm.$nextTick(done);
+        });
+
+        it('should be true', () => {
+          expect(vm.showMergePipelineForkWarning).toEqual(true);
+        });
+      });
+    });
+
+    describe('showTargetBranchAdvancedError', () => {
+      describe(`when the pipeline's target_sha property doesn't exist`, () => {
+        beforeEach(done => {
+          Vue.set(vm.mr.pipeline, 'target_sha', undefined);
+          Vue.set(vm.mr, 'targetBranchSha', 'abcd');
+          vm.$nextTick(done);
+        });
+
+        it('should be false', () => {
+          expect(vm.showTargetBranchAdvancedError).toEqual(false);
+        });
+      });
+
+      describe(`when the pipeline's target_sha matches the target branch's sha`, () => {
+        beforeEach(done => {
+          Vue.set(vm.mr.pipeline, 'target_sha', 'abcd');
+          Vue.set(vm.mr, 'targetBranchSha', 'abcd');
+          vm.$nextTick(done);
+        });
+
+        it('should be false', () => {
+          expect(vm.showTargetBranchAdvancedError).toEqual(false);
+        });
+      });
+
+      describe(`when the pipeline's target_sha does not match the target branch's sha`, () => {
+        beforeEach(done => {
+          Vue.set(vm.mr.pipeline, 'target_sha', 'abcd');
+          Vue.set(vm.mr, 'targetBranchSha', 'bcde');
+          vm.$nextTick(done);
+        });
+
+        it('should be true', () => {
+          expect(vm.showTargetBranchAdvancedError).toEqual(true);
+        });
+      });
+    });
   });
 
   describe('methods', () => {