Merge branch '229268-add-dismiss-button-to-suggest-pipeline' into 'master'

Add dismiss button to suggest pipeline

Closes #229268

See merge request gitlab-org/gitlab!36993

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

@@ -3,6 +3,7 @@ import { GlLink, GlSprintf, GlButton } from '@gitlab/ui';
 import MrWidgetIcon from './mr_widget_icon.vue';
 import Tracking from '~/tracking';
 import { s__ } from '~/locale';
+import DismissibleContainer from '~/vue_shared/components/dismissible_container.vue';
 
 const trackingMixin = Tracking.mixin();
 const TRACK_LABEL = 'no_pipeline_noticed';
@@ -24,6 +25,7 @@ export default {
     GlSprintf,
     GlButton,
     MrWidgetIcon,
+    DismissibleContainer,
   },
   mixins: [trackingMixin],
   props: {
@@ -39,6 +41,14 @@ export default {
       type: String,
       required: true,
     },
+    userCalloutsPath: {
+      type: String,
+      required: true,
+    },
+    userCalloutFeatureId: {
+      type: String,
+      required: true,
+    },
   },
   computed: {
     tracking() {
@@ -54,8 +64,13 @@ export default {
 };
 </script>
 <template>
-  <div class="mr-widget-body mr-pipeline-suggest gl-mb-3">
-    <div class="gl-display-flex gl-align-items-center">
+  <dismissible-container
+    class="mr-widget-body mr-pipeline-suggest gl-mb-3"
+    :path="userCalloutsPath"
+    :feature-id="userCalloutFeatureId"
+    @dismiss="$emit('dismiss')"
+  >
+    <template #title>
       <mr-widget-icon :name="$options.iconName" />
       <div>
         <gl-sprintf
@@ -85,9 +100,9 @@ export default {
           </template>
         </gl-sprintf>
       </div>
-    </div>
+    </template>
     <div class="row">
-      <div class="col-md-5 order-md-last col-12 gl-mt-5 mt-md-n3 svg-content svg-225">
+      <div class="col-md-5 order-md-last col-12 gl-mt-5 mt-md-n1 pt-md-1 svg-content svg-225">
         <img data-testid="pipeline-image" :src="pipelineSvgPath" />
       </div>
       <div class="col-md-7 order-md-first col-12">
@@ -124,5 +139,5 @@ export default {
         </div>
       </div>
     </div>
-  </div>
+  </dismissible-container>
 </template>

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -116,7 +116,12 @@ export default {
       return this.mr.hasCI || this.hasPipelineMustSucceedConflict;
     },
     shouldSuggestPipelines() {
-      return gon.features?.suggestPipeline && !this.mr.hasCI && this.mr.mergeRequestAddCiConfigPath;
+      return (
+        gon.features?.suggestPipeline &&
+        !this.mr.hasCI &&
+        this.mr.mergeRequestAddCiConfigPath &&
+        !this.mr.isDismissedSuggestPipeline
+      );
     },
     shouldRenderCodeQuality() {
       return this.mr?.codeclimate?.head_path;
@@ -374,6 +379,9 @@ export default {
         this.stopPolling();
       });
     },
+    dismissSuggestPipelines() {
+      this.mr.isDismissedSuggestPipeline = true;
+    },
   },
 };
 </script>
@@ -382,10 +390,14 @@ export default {
     <mr-widget-header :mr="mr" />
     <mr-widget-suggest-pipeline
       v-if="shouldSuggestPipelines"
+      data-testid="mr-suggest-pipeline"
       class="mr-widget-workflow"
       :pipeline-path="mr.mergeRequestAddCiConfigPath"
       :pipeline-svg-path="mr.pipelinesEmptySvgPath"
       :human-access="mr.humanAccess.toLowerCase()"
+      :user-callouts-path="mr.userCalloutsPath"
+      :user-callout-feature-id="mr.suggestPipelineFeatureId"
+      @dismiss="dismissSuggestPipelines"
     />
     <mr-widget-pipeline-container
       v-if="shouldRenderPipelines"

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

@@ -194,6 +194,9 @@ export default class MergeRequestStore {
     this.pipelinesEmptySvgPath = data.pipelines_empty_svg_path;
     this.humanAccess = data.human_access;
     this.newPipelinePath = data.new_project_pipeline_path;
+    this.userCalloutsPath = data.user_callouts_path;
+    this.suggestPipelineFeatureId = data.suggest_pipeline_feature_id;
+    this.isDismissedSuggestPipeline = data.is_dismissed_suggest_pipeline;
 
     // codeclimate
     const blobPath = data.blob_path || {};

app/assets/javascripts/vue_shared/components/dismissible_container.vue

+<script>
+import axios from '~/lib/utils/axios_utils';
+import { GlIcon } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlIcon,
+  },
+  props: {
+    path: {
+      type: String,
+      required: true,
+    },
+    featureId: {
+      type: String,
+      required: true,
+    },
+  },
+  methods: {
+    dismiss() {
+      axios
+        .post(this.path, {
+          feature_name: this.featureId,
+        })
+        .catch(e => {
+          // eslint-disable-next-line @gitlab/require-i18n-strings, no-console
+          console.error('Failed to dismiss message.', e);
+        });
+
+      this.$emit('dismiss');
+    },
+  },
+};
+</script>
+
+<template>
+  <div>
+    <div class="gl-display-flex gl-align-items-center">
+      <slot name="title"></slot>
+      <div class="ml-auto">
+        <button
+          :aria-label="__('Close')"
+          class="btn-blank"
+          type="button"
+          data-testid="close"
+          @click="dismiss"
+        >
+          <gl-icon name="close" aria-hidden="true" class="gl-text-gray-700" />
+        </button>
+      </div>
+    </div>
+    <slot></slot>
+  </div>
+</template>

app/models/user_callout_enums.rb

@@ -18,7 +18,8 @@ module UserCalloutEnums
       tabs_position_highlight: 10,
       webhooks_moved: 13,
       admin_integrations_moved: 15,
-      personal_access_token_expiry: 21 # EE-only
+      personal_access_token_expiry: 21, # EE-only
+      suggest_pipeline: 22
     }
   end
 end

app/serializers/merge_request_widget_entity.rb

@@ -3,6 +3,8 @@
 class MergeRequestWidgetEntity < Grape::Entity
   include RequestAwareEntity
 
+  SUGGEST_PIPELINE = 'suggest_pipeline'
+
   expose :id
   expose :iid
 
@@ -64,6 +66,18 @@ class MergeRequestWidgetEntity < Grape::Entity
     )
   end
 
+  expose :user_callouts_path, if: -> (*) { Feature.enabled?(:suggest_pipeline) } do |merge_request|
+    user_callouts_path
+  end
+
+  expose :suggest_pipeline_feature_id, if: -> (*) { Feature.enabled?(:suggest_pipeline) } do |merge_request|
+    SUGGEST_PIPELINE
+  end
+
+  expose :is_dismissed_suggest_pipeline, if: -> (*) { Feature.enabled?(:suggest_pipeline) } do |merge_request|
+    current_user && current_user.dismissed_callout?(feature_name: SUGGEST_PIPELINE)
+  end
+
   expose :human_access do |merge_request|
     merge_request.project.team.human_max_access(current_user&.id)
   end

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

@@ -255,6 +255,9 @@ export default {
       :pipeline-path="mr.mergeRequestAddCiConfigPath"
       :pipeline-svg-path="mr.pipelinesEmptySvgPath"
       :human-access="mr.humanAccess.toLowerCase()"
+      :user-callouts-path="mr.userCalloutsPath"
+      :user-callout-feature-id="mr.suggestPipelineFeatureId"
+      @dismiss="dismissSuggestPipelines"
     />
     <mr-widget-pipeline-container
       v-if="shouldRenderPipelines"

spec/frontend/vue_mr_widget/components/mr_widget_suggest_pipeline_spec.js

-import { mount } from '@vue/test-utils';
+import { mount, shallowMount } from '@vue/test-utils';
 import { GlLink, GlSprintf } from '@gitlab/ui';
 import suggestPipelineComponent from '~/vue_merge_request_widget/components/mr_widget_suggest_pipeline.vue';
 import MrWidgetIcon from '~/vue_merge_request_widget/components/mr_widget_icon.vue';
+import dismissibleContainer from '~/vue_shared/components/dismissible_container.vue';
 import { mockTracking, triggerEvent, unmockTracking } from 'helpers/tracking_helper';
-import { popoverProps, iconName } from './pipeline_tour_mock_data';
+import { suggestProps, iconName } from './pipeline_tour_mock_data';
+import axios from '~/lib/utils/axios_utils';
+import MockAdapter from 'axios-mock-adapter';
 
 describe('MRWidgetSuggestPipeline', () => {
+  describe('template', () => {
     let wrapper;
+
+    afterEach(() => {
+      wrapper.destroy();
+    });
+
+    describe('core functionality', () => {
+      const findOkBtn = () => wrapper.find('[data-testid="ok"]');
       let trackingSpy;
+      let mockAxios;
 
       const mockTrackingOnWrapper = () => {
         unmockTracking();
@@ -15,11 +27,12 @@ describe('MRWidgetSuggestPipeline', () => {
       };
 
       beforeEach(() => {
+        mockAxios = new MockAdapter(axios);
         document.body.dataset.page = 'projects:merge_requests:show';
         trackingSpy = mockTracking('_category_', undefined, jest.spyOn);
 
         wrapper = mount(suggestPipelineComponent, {
-      propsData: popoverProps,
+          propsData: suggestProps,
           stubs: {
             GlSprintf,
           },
@@ -27,18 +40,15 @@ describe('MRWidgetSuggestPipeline', () => {
       });
 
       afterEach(() => {
-    wrapper.destroy();
         unmockTracking();
+        mockAxios.restore();
       });
 
-  describe('template', () => {
-    const findOkBtn = () => wrapper.find('[data-testid="ok"]');
-
       it('renders add pipeline file link', () => {
         const link = wrapper.find(GlLink);
 
         expect(link.exists()).toBe(true);
-      expect(link.attributes().href).toBe(popoverProps.pipelinePath);
+        expect(link.attributes().href).toBe(suggestProps.pipelinePath);
       });
 
       it('renders the expected text', () => {
@@ -63,7 +73,7 @@ describe('MRWidgetSuggestPipeline', () => {
 
         expect(button.exists()).toBe(true);
         expect(button.classes('btn-info')).toEqual(true);
-      expect(button.attributes('href')).toBe(popoverProps.pipelinePath);
+        expect(button.attributes('href')).toBe(suggestProps.pipelinePath);
       });
 
       it('renders the help link', () => {
@@ -77,7 +87,7 @@ describe('MRWidgetSuggestPipeline', () => {
         const image = wrapper.find('[data-testid="pipeline-image"]');
 
         expect(image.exists()).toBe(true);
-      expect(image.attributes().src).toBe(popoverProps.pipelineSvgPath);
+        expect(image.attributes().src).toBe(suggestProps.pipelineSvgPath);
       });
 
       describe('tracking', () => {
@@ -87,7 +97,7 @@ describe('MRWidgetSuggestPipeline', () => {
 
           expect(trackingSpy).toHaveBeenCalledWith(expectedCategory, expectedAction, {
             label: wrapper.vm.$options.trackLabel,
-          property: popoverProps.humanAccess,
+            property: suggestProps.humanAccess,
           });
         });
 
@@ -98,7 +108,7 @@ describe('MRWidgetSuggestPipeline', () => {
 
           expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_link', {
             label: wrapper.vm.$options.trackLabel,
-          property: popoverProps.humanAccess,
+            property: suggestProps.humanAccess,
             value: '30',
           });
         });
@@ -110,10 +120,29 @@ describe('MRWidgetSuggestPipeline', () => {
 
           expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
             label: wrapper.vm.$options.trackLabel,
-          property: popoverProps.humanAccess,
+            property: suggestProps.humanAccess,
             value: '10',
           });
         });
       });
     });
+
+    describe('dismissible', () => {
+      const findDismissContainer = () => wrapper.find(dismissibleContainer);
+
+      beforeEach(() => {
+        wrapper = shallowMount(suggestPipelineComponent, { propsData: suggestProps });
+      });
+
+      it('renders the dismissal container', () => {
+        expect(findDismissContainer().exists()).toBe(true);
+      });
+
+      it('emits dismiss upon dismissal button click', () => {
+        findDismissContainer().vm.$emit('dismiss');
+
+        expect(wrapper.emitted().dismiss).toBeTruthy();
+      });
+    });
+  });
 });

spec/frontend/vue_mr_widget/components/pipeline_tour_mock_data.js

-export const popoverProps = {
+export const suggestProps = {
   pipelinePath: '/foo/bar/add/pipeline/path',
   pipelineSvgPath: 'assets/illustrations/something.svg',
   humanAccess: 'maintainer',
+  userCalloutsPath: 'some/callout/path',
+  userCalloutFeatureId: 'suggest_pipeline',
 };
 
 export const iconName = 'status_notfound';

spec/frontend/vue_mr_widget/mock_data.js

@@ -37,6 +37,9 @@ export default {
   target_project_id: 19,
   target_project_full_path: '/group2/project2',
   merge_request_add_ci_config_path: '/group2/project2/new/pipeline',
+  is_dismissed_suggest_pipeline: false,
+  user_callouts_path: 'some/callout/path',
+  suggest_pipeline_feature_id: 'suggest_pipeline',
   new_project_pipeline_path: '/group2/project2/pipelines/new',
   metrics: {
     merged_by: {

spec/frontend/vue_mr_widget/mr_widget_options_spec.js

@@ -62,6 +62,9 @@ describe('mrWidgetOptions', () => {
     return axios.waitForAll();
   };
 
+  const findSuggestPipeline = () => vm.$el.querySelector('[data-testid="mr-suggest-pipeline"]');
+  const findSuggestPipelineButton = () => findSuggestPipeline().querySelector('button');
+
   describe('default', () => {
     beforeEach(() => {
       return createComponent();
@@ -804,42 +807,48 @@ describe('mrWidgetOptions', () => {
       });
     });
 
-    it('should not suggest pipelines', () => {
-      vm.mr.mergeRequestAddCiConfigPath = null;
-
-      expect(vm.shouldSuggestPipelines).toBeFalsy();
+    it('should not suggest pipelines when feature flag is not present', () => {
+      expect(findSuggestPipeline()).toBeNull();
     });
   });
 
   describe('given suggestPipeline feature flag is enabled', () => {
     beforeEach(() => {
+      mock.onAny().reply(200);
+
       // This is needed because some grandchildren Bootstrap components throw warnings
       // https://gitlab.com/gitlab-org/gitlab/issues/208458
       jest.spyOn(console, 'warn').mockImplementation();
 
       gon.features = { suggestPipeline: true };
-      return createComponent();
-    });
 
-    it('should suggest pipelines when none exist', () => {
-      vm.mr.mergeRequestAddCiConfigPath = 'some/path';
+      createComponent();
+
       vm.mr.hasCI = false;
+    });
 
-      expect(vm.shouldSuggestPipelines).toBeTruthy();
+    it('should suggest pipelines when none exist', () => {
+      expect(findSuggestPipeline()).toEqual(expect.any(Element));
     });
 
-    it('should not suggest pipelines when they exist', () => {
-      vm.mr.mergeRequestAddCiConfigPath = null;
-      vm.mr.hasCI = false;
+    it.each([
+      { isDismissedSuggestPipeline: true },
+      { mergeRequestAddCiConfigPath: null },
+      { hasCI: true },
+    ])('with %s, should not suggest pipeline', async obj => {
+      Object.assign(vm.mr, obj);
 
-      expect(vm.shouldSuggestPipelines).toBeFalsy();
+      await vm.$nextTick();
+
+      expect(findSuggestPipeline()).toBeNull();
     });
 
-    it('should not suggest pipelines hasCI is true', () => {
-      vm.mr.mergeRequestAddCiConfigPath = 'some/path';
-      vm.mr.hasCI = true;
+    it('should allow dismiss of the suggest pipeline message', async () => {
+      findSuggestPipelineButton().click();
+
+      await vm.$nextTick();
 
-      expect(vm.shouldSuggestPipelines).toBeFalsy();
+      expect(findSuggestPipeline()).toBeNull();
     });
   });
 });

spec/frontend/vue_shared/components/dismissible_container_spec.js

+import MockAdapter from 'axios-mock-adapter';
+import axios from '~/lib/utils/axios_utils';
+import { shallowMount } from '@vue/test-utils';
+import dismissibleContainer from '~/vue_shared/components/dismissible_container.vue';
+
+describe('DismissibleContainer', () => {
+  let wrapper;
+  const propsData = {
+    path: 'some/path',
+    featureId: 'some-feature-id',
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('template', () => {
+    const findBtn = () => wrapper.find('[data-testid="close"]');
+    let mockAxios;
+
+    beforeEach(() => {
+      mockAxios = new MockAdapter(axios);
+      wrapper = shallowMount(dismissibleContainer, { propsData });
+    });
+
+    afterEach(() => {
+      mockAxios.restore();
+    });
+
+    it('successfully dismisses', () => {
+      mockAxios.onPost(propsData.path).replyOnce(200);
+      const button = findBtn();
+
+      button.trigger('click');
+
+      expect(wrapper.emitted().dismiss).toBeTruthy();
+    });
+  });
+
+  describe('slots', () => {
+    const slots = {
+      title: 'Foo Title',
+      default: 'default slot',
+    };
+
+    it.each(Object.keys(slots))('renders the %s slot', slot => {
+      const slotContent = slots[slot];
+      wrapper = shallowMount(dismissibleContainer, {
+        propsData,
+        slots: {
+          [slot]: `<span>${slotContent}</span>`,
+        },
+      });
+
+      expect(wrapper.text()).toContain(slotContent);
+    });
+  });
+});

spec/serializers/merge_request_widget_entity_spec.rb

@@ -256,6 +256,62 @@ RSpec.describe MergeRequestWidgetEntity do
     end
   end
 
+  describe 'user callouts' do
+    context 'when suggest pipeline feature is enabled' do
+      before do
+        stub_feature_flags(suggest_pipeline: true)
+      end
+
+      it 'provides a valid path value for user callout path' do
+        expect(subject[:user_callouts_path]).to eq '/-/user_callouts'
+      end
+
+      it 'provides a valid value for suggest pipeline feature id' do
+        expect(subject[:suggest_pipeline_feature_id]).to eq described_class::SUGGEST_PIPELINE
+      end
+
+      it 'provides a valid value for if it is dismissed' do
+        expect(subject[:is_dismissed_suggest_pipeline]).to be(false)
+      end
+
+      context 'when the suggest pipeline has been dismissed' do
+        before do
+          create(:user_callout, user: user, feature_name: described_class::SUGGEST_PIPELINE)
+        end
+
+        it 'indicates suggest pipeline has been dismissed' do
+          expect(subject[:is_dismissed_suggest_pipeline]).to be(true)
+        end
+      end
+
+      context 'when user is not logged in' do
+        let(:request) { double('request', current_user: nil, project: project) }
+
+        it 'returns a blank is dismissed value' do
+          expect(subject[:is_dismissed_suggest_pipeline]).to be_nil
+        end
+      end
+    end
+
+    context 'when suggest pipeline feature is not enabled' do
+      before do
+        stub_feature_flags(suggest_pipeline: false)
+      end
+
+      it 'provides no valid value for user callout path' do
+        expect(subject[:user_callouts_path]).to be_nil
+      end
+
+      it 'provides no valid value for suggest pipeline feature id' do
+        expect(subject[:suggest_pipeline_feature_id]).to be_nil
+      end
+
+      it 'provides no valid value for if it is dismissed' do
+        expect(subject[:is_dismissed_suggest_pipeline]).to be_nil
+      end
+    end
+  end
+
   it 'has human access' do
     project.add_maintainer(user)