Commit 01fe6eb8 authored by Ezekiel Kigbo's avatar Ezekiel Kigbo

Merge branch '223110-engineering-nudge-4-design-update' into 'master'

Add button to return to merge request on pipeline celebration

Closes #230497 and #223110

See merge request gitlab-org/gitlab!37393
parents b31cee4e a1569fe6
<script> <script>
import { GlModal, GlSprintf, GlLink } from '@gitlab/ui'; import { GlModal, GlSprintf, GlLink, GlButton } from '@gitlab/ui';
import Cookies from 'js-cookie'; import Cookies from 'js-cookie';
import { sprintf, s__, __ } from '~/locale'; import { sprintf, s__, __ } from '~/locale';
import { glEmojiTag } from '~/emoji'; import { glEmojiTag } from '~/emoji';
...@@ -18,6 +18,8 @@ export default { ...@@ -18,6 +18,8 @@ export default {
helpMessage: s__( helpMessage: s__(
`MR widget|Take a look at our %{beginnerLinkStart}Beginner's Guide to Continuous Integration%{beginnerLinkEnd} and our %{exampleLinkStart}examples of GitLab CI/CD%{exampleLinkEnd} to learn more.`, `MR widget|Take a look at our %{beginnerLinkStart}Beginner's Guide to Continuous Integration%{beginnerLinkEnd} and our %{exampleLinkStart}examples of GitLab CI/CD%{exampleLinkEnd} to learn more.`,
), ),
pipelinesButton: s__('MR widget|See your pipeline in action'),
mergeRequestButton: s__('MR widget|Back to the Merge request'),
modalTitle: sprintf( modalTitle: sprintf(
__("That's it, well done!%{celebrate}"), __("That's it, well done!%{celebrate}"),
{ {
...@@ -25,11 +27,13 @@ export default { ...@@ -25,11 +27,13 @@ export default {
}, },
false, false,
), ),
goToTrackValue: 10, goToTrackValuePipelines: 10,
goToTrackValueMergeRequest: 20,
trackEvent: 'click_button', trackEvent: 'click_button',
components: { components: {
GlModal, GlModal,
GlSprintf, GlSprintf,
GlButton,
GlLink, GlLink,
}, },
mixins: [trackingMixin], mixins: [trackingMixin],
...@@ -38,6 +42,11 @@ export default { ...@@ -38,6 +42,11 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
projectMergeRequestsPath: {
type: String,
required: false,
default: '',
},
commitCookie: { commitCookie: {
type: String, type: String,
required: true, required: true,
...@@ -59,6 +68,15 @@ export default { ...@@ -59,6 +68,15 @@ export default {
property: this.humanAccess, property: this.humanAccess,
}; };
}, },
goToMergeRequestPath() {
return this.commitCookiePath || this.projectMergeRequestsPath;
},
commitCookiePath() {
const cookieVal = Cookies.get(this.commitCookie);
if (cookieVal !== 'true') return cookieVal;
return '';
},
}, },
mounted() { mounted() {
this.track(); this.track();
...@@ -100,17 +118,28 @@ export default { ...@@ -100,17 +118,28 @@ export default {
</template> </template>
</gl-sprintf> </gl-sprintf>
<template #modal-footer> <template #modal-footer>
<a <gl-button
ref="goto" v-if="projectMergeRequestsPath"
ref="goToMergeRequest"
:href="goToMergeRequestPath"
:data-track-property="humanAccess"
:data-track-value="$options.goToTrackValueMergeRequest"
:data-track-event="$options.trackEvent"
:data-track-label="trackLabel"
>
{{ $options.mergeRequestButton }}
</gl-button>
<gl-button
ref="goToPipelines"
:href="goToPipelinesPath" :href="goToPipelinesPath"
class="btn btn-success" variant="success"
:data-track-property="humanAccess" :data-track-property="humanAccess"
:data-track-value="$options.goToTrackValue" :data-track-value="$options.goToTrackValuePipelines"
:data-track-event="$options.trackEvent" :data-track-event="$options.trackEvent"
:data-track-label="trackLabel" :data-track-label="trackLabel"
> >
{{ __('See your pipeline in action') }} {{ $options.pipelinesButton }}
</a> </gl-button>
</template> </template>
</gl-modal> </gl-modal>
</template> </template>
...@@ -50,6 +50,10 @@ export default { ...@@ -50,6 +50,10 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
mergeRequestPath: {
type: String,
required: true,
},
}, },
data() { data() {
return { return {
......
...@@ -10,6 +10,7 @@ export default el => ...@@ -10,6 +10,7 @@ export default el =>
target: el.dataset.target, target: el.dataset.target,
trackLabel: el.dataset.trackLabel, trackLabel: el.dataset.trackLabel,
dismissKey: el.dataset.dismissKey, dismissKey: el.dataset.dismissKey,
mergeRequestPath: el.dataset.mergeRequestPath,
humanAccess: el.dataset.humanAccess, humanAccess: el.dataset.humanAccess,
}, },
}); });
......
...@@ -67,12 +67,15 @@ export default () => { ...@@ -67,12 +67,15 @@ export default () => {
if (commitButton) { if (commitButton) {
const { dismissKey, humanAccess } = suggestEl.dataset; const { dismissKey, humanAccess } = suggestEl.dataset;
const urlParams = new URLSearchParams(window.location.search);
const mergeRequestPath = urlParams.get('mr_path') || true;
const commitCookieName = `suggest_gitlab_ci_yml_commit_${dismissKey}`; const commitCookieName = `suggest_gitlab_ci_yml_commit_${dismissKey}`;
const commitTrackLabel = 'suggest_gitlab_ci_yml_commit_changes'; const commitTrackLabel = 'suggest_gitlab_ci_yml_commit_changes';
const commitTrackValue = '20'; const commitTrackValue = '20';
commitButton.addEventListener('click', () => { commitButton.addEventListener('click', () => {
setCookie(commitCookieName, true); setCookie(commitCookieName, mergeRequestPath);
Tracking.event(undefined, 'click_button', { Tracking.event(undefined, 'click_button', {
label: commitTrackLabel, label: commitTrackLabel,
......
...@@ -62,6 +62,7 @@ class MergeRequestWidgetEntity < Grape::Entity ...@@ -62,6 +62,7 @@ class MergeRequestWidgetEntity < Grape::Entity
merge_request.source_branch, merge_request.source_branch,
file_name: '.gitlab-ci.yml', file_name: '.gitlab-ci.yml',
commit_message: s_("CommitMessage|Add %{file_name}") % { file_name: Gitlab::FileDetector::PATTERNS[:gitlab_ci] }, commit_message: s_("CommitMessage|Add %{file_name}") % { file_name: Gitlab::FileDetector::PATTERNS[:gitlab_ci] },
mr_path: merge_request_path(merge_request),
suggest_gitlab_ci_yml: true suggest_gitlab_ci_yml: true
) )
end end
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
.js-suggest-gitlab-ci-yml{ data: { toggle: 'popover', .js-suggest-gitlab-ci-yml{ data: { toggle: 'popover',
target: '#gitlab-ci-yml-selector', target: '#gitlab-ci-yml-selector',
track_label: 'suggest_gitlab_ci_yml', track_label: 'suggest_gitlab_ci_yml',
merge_request_path: params[:mr_path],
dismiss_key: @project.id, dismiss_key: @project.id,
human_access: human_access } } human_access: human_access } }
......
.js-success-pipeline-modal{ data: { 'commit-cookie': suggest_pipeline_commit_cookie_name, .js-success-pipeline-modal{ data: { 'commit-cookie': suggest_pipeline_commit_cookie_name,
'go-to-pipelines-path': project_pipelines_path(@project), 'go-to-pipelines-path': project_pipelines_path(@project),
'project-merge-requests-path': project_merge_requests_path(@project),
'human-access': @project.team.human_max_access(current_user&.id) } } 'human-access': @project.team.human_max_access(current_user&.id) } }
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
- if should_suggest_gitlab_ci_yml? - if should_suggest_gitlab_ci_yml?
.js-suggest-gitlab-ci-yml-commit-changes{ data: { toggle: 'popover', .js-suggest-gitlab-ci-yml-commit-changes{ data: { toggle: 'popover',
target: '#commit-changes', target: '#commit-changes',
merge_request_path: params[:mr_path],
track_label: 'suggest_commit_first_project_gitlab_ci_yml', track_label: 'suggest_commit_first_project_gitlab_ci_yml',
dismiss_key: @project.id, dismiss_key: @project.id,
human_access: human_access } } human_access: human_access } }
...@@ -14702,6 +14702,12 @@ msgstr "" ...@@ -14702,6 +14702,12 @@ msgstr ""
msgid "MERGED" msgid "MERGED"
msgstr "" msgstr ""
msgid "MR widget|Back to the Merge request"
msgstr ""
msgid "MR widget|See your pipeline in action"
msgstr ""
msgid "MR widget|Take a look at our %{beginnerLinkStart}Beginner's Guide to Continuous Integration%{beginnerLinkEnd} and our %{exampleLinkStart}examples of GitLab CI/CD%{exampleLinkEnd} to learn more." msgid "MR widget|Take a look at our %{beginnerLinkStart}Beginner's Guide to Continuous Integration%{beginnerLinkEnd} and our %{exampleLinkStart}examples of GitLab CI/CD%{exampleLinkEnd} to learn more."
msgstr "" msgstr ""
...@@ -21957,9 +21963,6 @@ msgstr "" ...@@ -21957,9 +21963,6 @@ msgstr ""
msgid "See what's new at GitLab" msgid "See what's new at GitLab"
msgstr "" msgstr ""
msgid "See your pipeline in action"
msgstr ""
msgid "Select" msgid "Select"
msgstr "" msgstr ""
......
const modalProps = { const modalProps = {
goToPipelinesPath: 'some_pipeline_path', goToPipelinesPath: 'some_pipeline_path',
projectMergeRequestsPath: 'some_mr_path',
commitCookie: 'some_cookie', commitCookie: 'some_cookie',
humanAccess: 'maintainer', humanAccess: 'maintainer',
}; };
......
...@@ -10,10 +10,7 @@ describe('PipelineTourSuccessModal', () => { ...@@ -10,10 +10,7 @@ describe('PipelineTourSuccessModal', () => {
let cookieSpy; let cookieSpy;
let trackingSpy; let trackingSpy;
beforeEach(() => { const createComponent = () => {
document.body.dataset.page = 'projects:blob:show';
trackingSpy = mockTracking('_category_', undefined, jest.spyOn);
wrapper = shallowMount(pipelineTourSuccess, { wrapper = shallowMount(pipelineTourSuccess, {
propsData: modalProps, propsData: modalProps,
stubs: { stubs: {
...@@ -21,13 +18,49 @@ describe('PipelineTourSuccessModal', () => { ...@@ -21,13 +18,49 @@ describe('PipelineTourSuccessModal', () => {
GlSprintf, GlSprintf,
}, },
}); });
};
beforeEach(() => {
document.body.dataset.page = 'projects:blob:show';
trackingSpy = mockTracking('_category_', undefined, jest.spyOn);
cookieSpy = jest.spyOn(Cookies, 'remove'); cookieSpy = jest.spyOn(Cookies, 'remove');
createComponent();
}); });
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
unmockTracking(); unmockTracking();
Cookies.remove(modalProps.commitCookie);
});
describe('when the commitCookie contains the mr path', () => {
const expectedMrPath = 'expected_mr_path';
beforeEach(() => {
Cookies.set(modalProps.commitCookie, expectedMrPath);
createComponent();
});
it('renders the path from the commit cookie for back to the merge request button', () => {
const goToMrBtn = wrapper.find({ ref: 'goToMergeRequest' });
expect(goToMrBtn.attributes('href')).toBe(expectedMrPath);
});
});
describe('when the commitCookie does not contain mr path', () => {
const expectedMrPath = modalProps.projectMergeRequestsPath;
beforeEach(() => {
Cookies.set(modalProps.commitCookie, true);
createComponent();
});
it('renders the path from projectMergeRequestsPath for back to the merge request button', () => {
const goToMrBtn = wrapper.find({ ref: 'goToMergeRequest' });
expect(goToMrBtn.attributes('href')).toBe(expectedMrPath);
});
}); });
it('has expected structure', () => { it('has expected structure', () => {
...@@ -58,7 +91,7 @@ describe('PipelineTourSuccessModal', () => { ...@@ -58,7 +91,7 @@ describe('PipelineTourSuccessModal', () => {
it('send an event when go to pipelines is clicked', () => { it('send an event when go to pipelines is clicked', () => {
trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn); trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
const goToBtn = wrapper.find({ ref: 'goto' }); const goToBtn = wrapper.find({ ref: 'goToPipelines' });
triggerEvent(goToBtn.element); triggerEvent(goToBtn.element);
expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', { expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
...@@ -67,5 +100,17 @@ describe('PipelineTourSuccessModal', () => { ...@@ -67,5 +100,17 @@ describe('PipelineTourSuccessModal', () => {
value: '10', value: '10',
}); });
}); });
it('sends an event when back to the merge request is clicked', () => {
trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
const goToBtn = wrapper.find({ ref: 'goToMergeRequest' });
triggerEvent(goToBtn.element);
expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
label: 'congratulate_first_pipeline',
property: modalProps.humanAccess,
value: '20',
});
});
}); });
}); });
...@@ -16,6 +16,7 @@ const commitTrackLabel = 'suggest_commit_first_project_gitlab_ci_yml'; ...@@ -16,6 +16,7 @@ const commitTrackLabel = 'suggest_commit_first_project_gitlab_ci_yml';
const dismissCookie = 'suggest_gitlab_ci_yml_99'; const dismissCookie = 'suggest_gitlab_ci_yml_99';
const humanAccess = 'owner'; const humanAccess = 'owner';
const mergeRequestPath = '/some/path';
describe('Suggest gitlab-ci.yml Popover', () => { describe('Suggest gitlab-ci.yml Popover', () => {
let wrapper; let wrapper;
...@@ -26,6 +27,7 @@ describe('Suggest gitlab-ci.yml Popover', () => { ...@@ -26,6 +27,7 @@ describe('Suggest gitlab-ci.yml Popover', () => {
target, target,
trackLabel, trackLabel,
dismissKey, dismissKey,
mergeRequestPath,
humanAccess, humanAccess,
}, },
stubs: { stubs: {
......
...@@ -43,7 +43,8 @@ describe('BlobBundle', () => { ...@@ -43,7 +43,8 @@ describe('BlobBundle', () => {
data-target="#target" data-target="#target"
data-track-label="suggest_gitlab_ci_yml" data-track-label="suggest_gitlab_ci_yml"
data-dismiss-key="1" data-dismiss-key="1"
data-human-access="owner"> data-human-access="owner"
data-merge-request-path="path/to/mr">
<button id='commit-changes' class="js-commit-button"></button> <button id='commit-changes' class="js-commit-button"></button>
<a class="btn btn-cancel" href="#"></a> <a class="btn btn-cancel" href="#"></a>
</div> </div>
......
...@@ -136,9 +136,22 @@ RSpec.describe MergeRequestWidgetEntity do ...@@ -136,9 +136,22 @@ RSpec.describe MergeRequestWidgetEntity do
let(:role) { :developer } let(:role) { :developer }
it 'has add ci config path' do it 'has add ci config path' do
expected_path = "/#{resource.project.full_path}/-/new/#{resource.source_branch}?commit_message=Add+.gitlab-ci.yml&file_name=.gitlab-ci.yml&suggest_gitlab_ci_yml=true" expected_path = "/#{resource.project.full_path}/-/new/#{resource.source_branch}"
expect(subject[:merge_request_add_ci_config_path]).to eq(expected_path) expect(subject[:merge_request_add_ci_config_path]).to include(expected_path)
end
it 'has expected params' do
expected_params = {
commit_message: 'Add .gitlab-ci.yml',
file_name: '.gitlab-ci.yml',
suggest_gitlab_ci_yml: 'true',
mr_path: "/#{resource.project.full_path}/-/merge_requests/#{resource.iid}"
}.with_indifferent_access
uri = Addressable::URI.parse(subject[:merge_request_add_ci_config_path])
expect(uri.query_values).to match(expected_params)
end end
context 'when auto devops is enabled' do context 'when auto devops is enabled' do
......
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