Commit a1569fe6 authored by Jackie Fraser's avatar Jackie Fraser Committed by Ezekiel Kigbo

Add button to return to MR on pipeline celebration

The pipeline suggest 1st nudge starts on the merge request, so:

- the 1st nudge was changed to pass the merge request path from
the 1st to the 2nd/3rd nudge.

- the 3rd nudge was changed to set the Merge request path into
the cookie set when clicking the Commit Changes button.

- the 4th nudge then checks for the cookie name instead of just
the cookie existence. The merge request path is used for the new
button. If they happened to have an old cookie, it links to the
project merge request list instead.
parent 787ad7eb
<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>
...@@ -49,6 +49,10 @@ export default { ...@@ -49,6 +49,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,
}, },
}); });
......
...@@ -65,12 +65,15 @@ export default () => { ...@@ -65,12 +65,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 } }
...@@ -14666,6 +14666,12 @@ msgstr "" ...@@ -14666,6 +14666,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 ""
...@@ -21891,9 +21897,6 @@ msgstr "" ...@@ -21891,9 +21897,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