Merge branch 'ph/widgetAlertMessages' into 'master'

Move merge request widget messages

See merge request gitlab-org/gitlab!61796

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

 <script>
-import { GlLink, GlIcon } from '@gitlab/ui';
-import { WARNING, DANGER, WARNING_MESSAGE_CLASS, DANGER_MESSAGE_CLASS } from '../constants';
+import { GlLink, GlAlert } from '@gitlab/ui';
 
 export default {
-  name: 'MrWidgetAlertMessage',
+  name: 'MRWidgetAlertMessage',
   components: {
+    GlAlert,
     GlLink,
-    GlIcon,
   },
   props: {
     type: {
       type: String,
-      required: false,
-      default: DANGER,
-      validator: (value) => [WARNING, DANGER].includes(value),
+      required: true,
     },
     helpPath: {
       type: String,
       required: false,
       default: undefined,
     },
+    dismissible: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
-  computed: {
-    messageClass() {
-      if (this.type === WARNING) {
-        return WARNING_MESSAGE_CLASS;
-      } else if (this.type === DANGER) {
-        return DANGER_MESSAGE_CLASS;
-      }
-
-      return '';
+  data() {
+    return {
+      isDismissed: false,
+    };
+  },
+  methods: {
+    onDismiss() {
+      this.isDismissed = true;
     },
   },
 };
 </script>
 
 <template>
-  <div class="gl-m-3 gl-ml-7" :class="messageClass">
+  <gl-alert v-if="!isDismissed" :variant="type" :dismissible="dismissible" @dismiss="onDismiss">
     <slot></slot>
-    <gl-link v-if="helpPath" :href="helpPath" target="_blank">
-      <gl-icon :size="16" name="question-o" class="align-middle" />
+    <gl-link v-if="helpPath" :href="helpPath" target="_blank" class="gl-label-link">
+      <slot name="link-content"></slot>
     </gl-link>
-  </div>
+  </gl-alert>
 </template>

app/assets/javascripts/vue_merge_request_widget/constants.js

@@ -5,9 +5,6 @@ export const WARNING = 'warning';
 export const DANGER = 'danger';
 export const INFO = 'info';
 
-export const WARNING_MESSAGE_CLASS = 'warning_message';
-export const DANGER_MESSAGE_CLASS = 'danger_message';
-
 export const MWPS_MERGE_STRATEGY = 'merge_when_pipeline_succeeds';
 export const MTWPS_MERGE_STRATEGY = 'add_to_merge_train_when_pipeline_succeeds';
 export const MT_MERGE_STRATEGY = 'merge_train';

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -198,6 +198,9 @@ export default {
     formattedHumanAccess() {
       return (this.mr.humanAccess || '').toLowerCase();
     },
+    hasAlerts() {
+      return this.mr.mergeError || this.showMergePipelineForkWarning;
+    },
   },
   watch: {
     state(newVal, oldVal) {
@@ -432,7 +435,14 @@ export default {
 </script>
 <template>
   <div v-if="isLoaded" class="mr-state-widget gl-mt-3">
-    <mr-widget-header :mr="mr" />
+    <header
+      class="gl-overflow-hidden gl-rounded-base gl-border-solid gl-border-1 gl-border-gray-100"
+    >
+      <mr-widget-alert-message v-if="shouldRenderCollaborationStatus" type="info">
+        {{ s__('mrWidget|Members who can merge are allowed to add commits.') }}
+      </mr-widget-alert-message>
+      <mr-widget-header :mr="mr" />
+    </header>
     <mr-widget-suggest-pipeline
       v-if="shouldSuggestPipelines"
       data-testid="mr-suggest-pipeline"
@@ -456,6 +466,25 @@ export default {
       :service="service"
     />
     <div class="mr-section-container mr-widget-workflow">
+      <div v-if="hasAlerts" class="gl-overflow-hidden mr-widget-alert-container">
+        <mr-widget-alert-message v-if="mr.mergeError" type="danger" dismissible>
+          <span v-safe-html="mergeError"></span>
+        </mr-widget-alert-message>
+        <mr-widget-alert-message
+          v-if="showMergePipelineForkWarning"
+          type="warning"
+          :help-path="mr.mergeRequestPipelinesHelpPath"
+        >
+          {{
+            s__(
+              'mrWidget|If the last pipeline ran in the fork project, it may be inaccurate. Before merge, we advise running a pipeline in this project.',
+            )
+          }}
+          <template #link-content>
+            {{ __('Learn more') }}
+          </template>
+        </mr-widget-alert-message>
+      </div>
       <!-- <extensions-container :mr="mr" /> -->
       <grouped-codequality-reports-app
         v-if="shouldRenderCodeQuality"
@@ -492,34 +521,12 @@ export default {
         <component :is="componentName" :mr="mr" :service="service" />
 
         <div class="mr-widget-info">
-          <section v-if="shouldRenderCollaborationStatus" class="mr-info-list mr-links">
-            <p>
-              {{ s__('mrWidget|Allows commits from members who can merge to the target branch') }}
-            </p>
-          </section>
-
           <mr-widget-related-links
             v-if="shouldRenderRelatedLinks"
             :state="mr.state"
             :related-links="mr.relatedLinks"
           />
 
-          <mr-widget-alert-message
-            v-if="showMergePipelineForkWarning"
-            type="warning"
-            :help-path="mr.mergeRequestPipelinesHelpPath"
-          >
-            {{
-              s__(
-                'mrWidget|If the last pipeline ran in the fork project, it may be inaccurate. Before merge, we advise running a pipeline in this project.',
-              )
-            }}
-          </mr-widget-alert-message>
-
-          <mr-widget-alert-message v-if="mr.mergeError" type="danger">
-            <span v-safe-html="mergeError"></span>
-          </mr-widget-alert-message>
-
           <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
         </div>
       </div>

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

@@ -44,7 +44,6 @@ export default class MergeRequestStore {
     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.mergeTrainWhenPipelineSucceedsDocsPath = data.merge_train_when_pipeline_succeeds_docs_path;
     this.commitMessage = data.default_merge_commit_message;
     this.shortMergeCommitSha = data.short_merged_commit_sha;

app/assets/stylesheets/pages/merge_requests.scss

@@ -43,8 +43,6 @@ $tabs-holder-z-index: 250;
 }
 
 .mr-widget-section {
-  border-radius: $border-radius-default $border-radius-default 0 0;
-
   .code-text {
     flex: 1;
   }
@@ -88,7 +86,6 @@ $tabs-holder-z-index: 250;
 .mr-section-container {
   border: 1px solid $border-color;
   border-radius: $border-radius-default;
-  border-top: 0;
   background: var(--white, $white);
 }
 
@@ -110,7 +107,6 @@ $tabs-holder-z-index: 250;
     border-radius: $border-radius-default;
   }
 
-  .mr-widget-section,
   .mr-widget-footer {
     border-top: solid 1px $border-color;
   }
@@ -525,9 +521,7 @@ $tabs-holder-z-index: 250;
 
 .mr-source-target {
   flex-wrap: wrap;
-  border-radius: $border-radius-default;
   padding: $gl-padding;
-  border: 1px solid $border-color;
   background: var(--white, $white);
   min-height: $mr-widget-min-height;
 
@@ -1027,3 +1021,13 @@ $tabs-holder-z-index: 250;
     vertical-align: middle;
   }
 }
+
+.mr-widget-alert-container {
+  $radius: $border-radius-default - 1px;
+
+  border-radius: $radius $radius 0 0;
+
+  .gl-alert:not(:last-child) {
+    margin-bottom: 1px;
+  }
+}

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

@@ -252,7 +252,14 @@ export default {
 </script>
 <template>
   <div v-if="isLoaded" class="mr-state-widget gl-mt-3">
-    <mr-widget-header :mr="mr" />
+    <header
+      class="gl-overflow-hidden gl-rounded-base gl-border-solid gl-border-1 gl-border-gray-100"
+    >
+      <mr-widget-alert-message v-if="shouldRenderCollaborationStatus" type="info">
+        {{ s__('mrWidget|Members who can merge are allowed to add commits.') }}
+      </mr-widget-alert-message>
+      <mr-widget-header :mr="mr" />
+    </header>
     <mr-widget-suggest-pipeline
       v-if="shouldSuggestPipelines"
       class="mr-widget-workflow"
@@ -275,6 +282,25 @@ export default {
       :service="service"
     />
     <div class="mr-section-container mr-widget-workflow">
+      <div v-if="hasAlerts" class="gl-overflow-hidden mr-widget-alert-container">
+        <mr-widget-alert-message v-if="mr.mergeError" type="danger" dismissible>
+          <span v-safe-html="mergeError"></span>
+        </mr-widget-alert-message>
+        <mr-widget-alert-message
+          v-if="showMergePipelineForkWarning"
+          type="warning"
+          :help-path="mr.mergeRequestPipelinesHelpPath"
+        >
+          {{
+            s__(
+              'mrWidget|If the last pipeline ran in the fork project, it may be inaccurate. Before merge, we advise running a pipeline in this project.',
+            )
+          }}
+          <template #link-content>
+            {{ __('Learn more') }}
+          </template>
+        </mr-widget-alert-message>
+      </div>
       <blocking-merge-requests-report :mr="mr" />
       <grouped-codequality-reports-app
         v-if="shouldRenderCodeQuality"
@@ -393,34 +419,12 @@ export default {
       <div class="mr-widget-section">
         <component :is="componentName" :mr="mr" :service="service" />
         <div class="mr-widget-info">
-          <section v-if="mr.allowCollaboration" class="mr-info-list gl-ml-7">
-            <p>
-              {{ s__('mrWidget|Allows commits from members who can merge to the target branch') }}
-            </p>
-          </section>
-
           <mr-widget-related-links
             v-if="shouldRenderRelatedLinks"
             :state="mr.state"
             :related-links="mr.relatedLinks"
           />
 
-          <mr-widget-alert-message
-            v-if="showMergePipelineForkWarning"
-            type="warning"
-            :help-path="mr.mergeRequestPipelinesHelpPath"
-          >
-            {{
-              s__(
-                'mrWidget|Fork project merge requests do not create merge request pipelines that validate a post merge result unless invoked by a project member.',
-              )
-            }}
-          </mr-widget-alert-message>
-
-          <mr-widget-alert-message v-if="mr.mergeError" type="danger">
-            <span v-safe-html="mergeError"></span>
-          </mr-widget-alert-message>
-
           <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
         </div>
       </div>

ee/spec/features/merge_request/user_sees_merge_widget_spec.rb

@@ -73,12 +73,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
       it 'shows a warning that fork project merge request does not create merge request pipelines by default', :sidekiq_might_not_need_inline do
         visit project_merge_request_path(project, merge_request)
 
-        within('.warning_message') do
-          expect(page)
-            .to have_content('Fork project merge requests do not create merge' \
-                             ' request pipelines that validate a post merge result' \
-                             ' unless invoked by a project member.')
-        end
+        expect(page).to have_content('If the last pipeline ran in the fork project, it may be inaccurate.')
       end
     end
   end

locale/gitlab.pot

@@ -38902,9 +38902,6 @@ msgstr ""
 msgid "mrWidget|Added to the merge train. There are %{mergeTrainPosition} merge requests waiting to be merged"
 msgstr ""
 
-msgid "mrWidget|Allows commits from members who can merge to the target branch"
-msgstr ""
-
 msgid "mrWidget|An error occurred while removing your approval."
 msgstr ""
 
@@ -38974,9 +38971,6 @@ msgstr ""
 msgid "mrWidget|Fast-forward merge is not possible. To merge this request, first rebase locally."
 msgstr ""
 
-msgid "mrWidget|Fork project merge requests do not create merge request pipelines that validate a post merge result unless invoked by a project member."
-msgstr ""
-
 msgid "mrWidget|If the %{missingBranchName} branch exists in your local repository, you can merge this merge request manually using the command line"
 msgstr ""
 
@@ -38992,6 +38986,9 @@ msgstr ""
 msgid "mrWidget|Mark as ready"
 msgstr ""
 
+msgid "mrWidget|Members who can merge are allowed to add commits."
+msgstr ""
+
 msgid "mrWidget|Mentions"
 msgstr ""
 

spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe 'create a merge request, allowing commits from members who can me
 
     wait_for_requests
 
-    expect(page).to have_content('Allows commits from members who can merge to the target branch')
+    expect(page).to have_content('Members who can merge are allowed to add commits.')
   end
 
   it 'shows a message when one of the projects is private', :sidekiq_might_not_need_inline do
@@ -59,7 +59,7 @@ RSpec.describe 'create a merge request, allowing commits from members who can me
 
       visit_new_merge_request
 
-      expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
+      expect(page).not_to have_content('The fork project allows commits from members who can write to the target branch.')
     end
   end
 
@@ -81,7 +81,7 @@ RSpec.describe 'create a merge request, allowing commits from members who can me
     it 'hides the option from members' do
       visit edit_project_merge_request_path(target_project, merge_request)
 
-      expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
+      expect(page).not_to have_content('The fork project allows commits from members who can write to the target branch.')
     end
   end
 end

spec/frontend/vue_mr_widget/components/mr_widget_alert_message_spec.js

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

spec/frontend/vue_mr_widget/mr_widget_options_spec.js

@@ -26,7 +26,7 @@ describe('MrWidgetOptions', () => {
   let wrapper;
   let mock;
 
-  const COLLABORATION_MESSAGE = 'Allows commits from members who can merge to the target branch';
+  const COLLABORATION_MESSAGE = 'Members who can merge are allowed to add commits';
 
   beforeEach(() => {
     gl.mrWidgetData = { ...mockData };