Commit ae03fb53 authored by Doug Stull's avatar Doug Stull Committed by James Lopez

Add dismiss functionality to suggest pipeline

- allow users to not be hassled constantly
parent 04a53f93
...@@ -3,6 +3,7 @@ import { GlLink, GlSprintf, GlButton } from '@gitlab/ui'; ...@@ -3,6 +3,7 @@ import { GlLink, GlSprintf, GlButton } from '@gitlab/ui';
import MrWidgetIcon from './mr_widget_icon.vue'; import MrWidgetIcon from './mr_widget_icon.vue';
import Tracking from '~/tracking'; import Tracking from '~/tracking';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import DismissibleContainer from '~/vue_shared/components/dismissible_container.vue';
const trackingMixin = Tracking.mixin(); const trackingMixin = Tracking.mixin();
const TRACK_LABEL = 'no_pipeline_noticed'; const TRACK_LABEL = 'no_pipeline_noticed';
...@@ -24,6 +25,7 @@ export default { ...@@ -24,6 +25,7 @@ export default {
GlSprintf, GlSprintf,
GlButton, GlButton,
MrWidgetIcon, MrWidgetIcon,
DismissibleContainer,
}, },
mixins: [trackingMixin], mixins: [trackingMixin],
props: { props: {
...@@ -39,6 +41,14 @@ export default { ...@@ -39,6 +41,14 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
userCalloutsPath: {
type: String,
required: true,
},
userCalloutFeatureId: {
type: String,
required: true,
},
}, },
computed: { computed: {
tracking() { tracking() {
...@@ -54,8 +64,13 @@ export default { ...@@ -54,8 +64,13 @@ export default {
}; };
</script> </script>
<template> <template>
<div class="mr-widget-body mr-pipeline-suggest gl-mb-3"> <dismissible-container
<div class="gl-display-flex gl-align-items-center"> 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" /> <mr-widget-icon :name="$options.iconName" />
<div> <div>
<gl-sprintf <gl-sprintf
...@@ -85,9 +100,9 @@ export default { ...@@ -85,9 +100,9 @@ export default {
</template> </template>
</gl-sprintf> </gl-sprintf>
</div> </div>
</div> </template>
<div class="row"> <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" /> <img data-testid="pipeline-image" :src="pipelineSvgPath" />
</div> </div>
<div class="col-md-7 order-md-first col-12"> <div class="col-md-7 order-md-first col-12">
...@@ -124,5 +139,5 @@ export default { ...@@ -124,5 +139,5 @@ export default {
</div> </div>
</div> </div>
</div> </div>
</div> </dismissible-container>
</template> </template>
...@@ -116,7 +116,12 @@ export default { ...@@ -116,7 +116,12 @@ export default {
return this.mr.hasCI || this.hasPipelineMustSucceedConflict; return this.mr.hasCI || this.hasPipelineMustSucceedConflict;
}, },
shouldSuggestPipelines() { 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() { shouldRenderCodeQuality() {
return this.mr?.codeclimate?.head_path; return this.mr?.codeclimate?.head_path;
...@@ -374,6 +379,9 @@ export default { ...@@ -374,6 +379,9 @@ export default {
this.stopPolling(); this.stopPolling();
}); });
}, },
dismissSuggestPipelines() {
this.mr.isDismissedSuggestPipeline = true;
},
}, },
}; };
</script> </script>
...@@ -382,10 +390,14 @@ export default { ...@@ -382,10 +390,14 @@ export default {
<mr-widget-header :mr="mr" /> <mr-widget-header :mr="mr" />
<mr-widget-suggest-pipeline <mr-widget-suggest-pipeline
v-if="shouldSuggestPipelines" v-if="shouldSuggestPipelines"
data-testid="mr-suggest-pipeline"
class="mr-widget-workflow" class="mr-widget-workflow"
:pipeline-path="mr.mergeRequestAddCiConfigPath" :pipeline-path="mr.mergeRequestAddCiConfigPath"
:pipeline-svg-path="mr.pipelinesEmptySvgPath" :pipeline-svg-path="mr.pipelinesEmptySvgPath"
:human-access="mr.humanAccess.toLowerCase()" :human-access="mr.humanAccess.toLowerCase()"
:user-callouts-path="mr.userCalloutsPath"
:user-callout-feature-id="mr.suggestPipelineFeatureId"
@dismiss="dismissSuggestPipelines"
/> />
<mr-widget-pipeline-container <mr-widget-pipeline-container
v-if="shouldRenderPipelines" v-if="shouldRenderPipelines"
......
...@@ -194,6 +194,9 @@ export default class MergeRequestStore { ...@@ -194,6 +194,9 @@ export default class MergeRequestStore {
this.pipelinesEmptySvgPath = data.pipelines_empty_svg_path; this.pipelinesEmptySvgPath = data.pipelines_empty_svg_path;
this.humanAccess = data.human_access; this.humanAccess = data.human_access;
this.newPipelinePath = data.new_project_pipeline_path; 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 // codeclimate
const blobPath = data.blob_path || {}; const blobPath = data.blob_path || {};
......
<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>
...@@ -18,7 +18,8 @@ module UserCalloutEnums ...@@ -18,7 +18,8 @@ module UserCalloutEnums
tabs_position_highlight: 10, tabs_position_highlight: 10,
webhooks_moved: 13, webhooks_moved: 13,
admin_integrations_moved: 15, admin_integrations_moved: 15,
personal_access_token_expiry: 21 # EE-only personal_access_token_expiry: 21, # EE-only
suggest_pipeline: 22
} }
end end
end end
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
class MergeRequestWidgetEntity < Grape::Entity class MergeRequestWidgetEntity < Grape::Entity
include RequestAwareEntity include RequestAwareEntity
SUGGEST_PIPELINE = 'suggest_pipeline'
expose :id expose :id
expose :iid expose :iid
...@@ -64,6 +66,18 @@ class MergeRequestWidgetEntity < Grape::Entity ...@@ -64,6 +66,18 @@ class MergeRequestWidgetEntity < Grape::Entity
) )
end 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| expose :human_access do |merge_request|
merge_request.project.team.human_max_access(current_user&.id) merge_request.project.team.human_max_access(current_user&.id)
end end
......
...@@ -255,6 +255,9 @@ export default { ...@@ -255,6 +255,9 @@ export default {
:pipeline-path="mr.mergeRequestAddCiConfigPath" :pipeline-path="mr.mergeRequestAddCiConfigPath"
:pipeline-svg-path="mr.pipelinesEmptySvgPath" :pipeline-svg-path="mr.pipelinesEmptySvgPath"
:human-access="mr.humanAccess.toLowerCase()" :human-access="mr.humanAccess.toLowerCase()"
:user-callouts-path="mr.userCalloutsPath"
:user-callout-feature-id="mr.suggestPipelineFeatureId"
@dismiss="dismissSuggestPipelines"
/> />
<mr-widget-pipeline-container <mr-widget-pipeline-container
v-if="shouldRenderPipelines" v-if="shouldRenderPipelines"
......
import { mount } from '@vue/test-utils'; import { mount, shallowMount } from '@vue/test-utils';
import { GlLink, GlSprintf } from '@gitlab/ui'; import { GlLink, GlSprintf } from '@gitlab/ui';
import suggestPipelineComponent from '~/vue_merge_request_widget/components/mr_widget_suggest_pipeline.vue'; 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 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 { 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('MRWidgetSuggestPipeline', () => {
describe('template', () => {
let wrapper; let wrapper;
afterEach(() => {
wrapper.destroy();
});
describe('core functionality', () => {
const findOkBtn = () => wrapper.find('[data-testid="ok"]');
let trackingSpy; let trackingSpy;
let mockAxios;
const mockTrackingOnWrapper = () => { const mockTrackingOnWrapper = () => {
unmockTracking(); unmockTracking();
...@@ -15,11 +27,12 @@ describe('MRWidgetSuggestPipeline', () => { ...@@ -15,11 +27,12 @@ describe('MRWidgetSuggestPipeline', () => {
}; };
beforeEach(() => { beforeEach(() => {
mockAxios = new MockAdapter(axios);
document.body.dataset.page = 'projects:merge_requests:show'; document.body.dataset.page = 'projects:merge_requests:show';
trackingSpy = mockTracking('_category_', undefined, jest.spyOn); trackingSpy = mockTracking('_category_', undefined, jest.spyOn);
wrapper = mount(suggestPipelineComponent, { wrapper = mount(suggestPipelineComponent, {
propsData: popoverProps, propsData: suggestProps,
stubs: { stubs: {
GlSprintf, GlSprintf,
}, },
...@@ -27,18 +40,15 @@ describe('MRWidgetSuggestPipeline', () => { ...@@ -27,18 +40,15 @@ describe('MRWidgetSuggestPipeline', () => {
}); });
afterEach(() => { afterEach(() => {
wrapper.destroy();
unmockTracking(); unmockTracking();
mockAxios.restore();
}); });
describe('template', () => {
const findOkBtn = () => wrapper.find('[data-testid="ok"]');
it('renders add pipeline file link', () => { it('renders add pipeline file link', () => {
const link = wrapper.find(GlLink); const link = wrapper.find(GlLink);
expect(link.exists()).toBe(true); expect(link.exists()).toBe(true);
expect(link.attributes().href).toBe(popoverProps.pipelinePath); expect(link.attributes().href).toBe(suggestProps.pipelinePath);
}); });
it('renders the expected text', () => { it('renders the expected text', () => {
...@@ -63,7 +73,7 @@ describe('MRWidgetSuggestPipeline', () => { ...@@ -63,7 +73,7 @@ describe('MRWidgetSuggestPipeline', () => {
expect(button.exists()).toBe(true); expect(button.exists()).toBe(true);
expect(button.classes('btn-info')).toEqual(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', () => { it('renders the help link', () => {
...@@ -77,7 +87,7 @@ describe('MRWidgetSuggestPipeline', () => { ...@@ -77,7 +87,7 @@ describe('MRWidgetSuggestPipeline', () => {
const image = wrapper.find('[data-testid="pipeline-image"]'); const image = wrapper.find('[data-testid="pipeline-image"]');
expect(image.exists()).toBe(true); expect(image.exists()).toBe(true);
expect(image.attributes().src).toBe(popoverProps.pipelineSvgPath); expect(image.attributes().src).toBe(suggestProps.pipelineSvgPath);
}); });
describe('tracking', () => { describe('tracking', () => {
...@@ -87,7 +97,7 @@ describe('MRWidgetSuggestPipeline', () => { ...@@ -87,7 +97,7 @@ describe('MRWidgetSuggestPipeline', () => {
expect(trackingSpy).toHaveBeenCalledWith(expectedCategory, expectedAction, { expect(trackingSpy).toHaveBeenCalledWith(expectedCategory, expectedAction, {
label: wrapper.vm.$options.trackLabel, label: wrapper.vm.$options.trackLabel,
property: popoverProps.humanAccess, property: suggestProps.humanAccess,
}); });
}); });
...@@ -98,7 +108,7 @@ describe('MRWidgetSuggestPipeline', () => { ...@@ -98,7 +108,7 @@ describe('MRWidgetSuggestPipeline', () => {
expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_link', { expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_link', {
label: wrapper.vm.$options.trackLabel, label: wrapper.vm.$options.trackLabel,
property: popoverProps.humanAccess, property: suggestProps.humanAccess,
value: '30', value: '30',
}); });
}); });
...@@ -110,10 +120,29 @@ describe('MRWidgetSuggestPipeline', () => { ...@@ -110,10 +120,29 @@ describe('MRWidgetSuggestPipeline', () => {
expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', { expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
label: wrapper.vm.$options.trackLabel, label: wrapper.vm.$options.trackLabel,
property: popoverProps.humanAccess, property: suggestProps.humanAccess,
value: '10', 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();
});
});
});
}); });
export const popoverProps = { export const suggestProps = {
pipelinePath: '/foo/bar/add/pipeline/path', pipelinePath: '/foo/bar/add/pipeline/path',
pipelineSvgPath: 'assets/illustrations/something.svg', pipelineSvgPath: 'assets/illustrations/something.svg',
humanAccess: 'maintainer', humanAccess: 'maintainer',
userCalloutsPath: 'some/callout/path',
userCalloutFeatureId: 'suggest_pipeline',
}; };
export const iconName = 'status_notfound'; export const iconName = 'status_notfound';
...@@ -37,6 +37,9 @@ export default { ...@@ -37,6 +37,9 @@ export default {
target_project_id: 19, target_project_id: 19,
target_project_full_path: '/group2/project2', target_project_full_path: '/group2/project2',
merge_request_add_ci_config_path: '/group2/project2/new/pipeline', 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', new_project_pipeline_path: '/group2/project2/pipelines/new',
metrics: { metrics: {
merged_by: { merged_by: {
......
...@@ -62,6 +62,9 @@ describe('mrWidgetOptions', () => { ...@@ -62,6 +62,9 @@ describe('mrWidgetOptions', () => {
return axios.waitForAll(); return axios.waitForAll();
}; };
const findSuggestPipeline = () => vm.$el.querySelector('[data-testid="mr-suggest-pipeline"]');
const findSuggestPipelineButton = () => findSuggestPipeline().querySelector('button');
describe('default', () => { describe('default', () => {
beforeEach(() => { beforeEach(() => {
return createComponent(); return createComponent();
...@@ -804,42 +807,48 @@ describe('mrWidgetOptions', () => { ...@@ -804,42 +807,48 @@ describe('mrWidgetOptions', () => {
}); });
}); });
it('should not suggest pipelines', () => { it('should not suggest pipelines when feature flag is not present', () => {
vm.mr.mergeRequestAddCiConfigPath = null; expect(findSuggestPipeline()).toBeNull();
expect(vm.shouldSuggestPipelines).toBeFalsy();
}); });
}); });
describe('given suggestPipeline feature flag is enabled', () => { describe('given suggestPipeline feature flag is enabled', () => {
beforeEach(() => { beforeEach(() => {
mock.onAny().reply(200);
// This is needed because some grandchildren Bootstrap components throw warnings // This is needed because some grandchildren Bootstrap components throw warnings
// https://gitlab.com/gitlab-org/gitlab/issues/208458 // https://gitlab.com/gitlab-org/gitlab/issues/208458
jest.spyOn(console, 'warn').mockImplementation(); jest.spyOn(console, 'warn').mockImplementation();
gon.features = { suggestPipeline: true }; gon.features = { suggestPipeline: true };
return createComponent();
});
it('should suggest pipelines when none exist', () => { createComponent();
vm.mr.mergeRequestAddCiConfigPath = 'some/path';
vm.mr.hasCI = false; 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', () => { it.each([
vm.mr.mergeRequestAddCiConfigPath = null; { isDismissedSuggestPipeline: true },
vm.mr.hasCI = false; { 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', () => { it('should allow dismiss of the suggest pipeline message', async () => {
vm.mr.mergeRequestAddCiConfigPath = 'some/path'; findSuggestPipelineButton().click();
vm.mr.hasCI = true;
await vm.$nextTick();
expect(vm.shouldSuggestPipelines).toBeFalsy(); expect(findSuggestPipeline()).toBeNull();
}); });
}); });
}); });
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);
});
});
});
...@@ -256,6 +256,62 @@ RSpec.describe MergeRequestWidgetEntity do ...@@ -256,6 +256,62 @@ RSpec.describe MergeRequestWidgetEntity do
end end
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 it 'has human access' do
project.add_maintainer(user) project.add_maintainer(user)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment