Skip to content

G
gitlab-ce
Commit 6aa1d555 authored by Lee Tickett's avatar Lee Tickett Committed by Enrique Alcántara
Browse files
Options

Fix job page copy source branch button 

The button was copying the source ref instead of branch.
As a bonus, we've also added the keyboard shortcut (b)
used to copy the source branch name on the merge request page.

Changelog: fixed
parent f4d7de21
Hide whitespace changes
Inline Side-by-side
Showing with 236 additions and 131 deletions
app/assets/javascripts/jobs/components/stages_dropdown.vue
View file @ 6aa1d555
<script>
import { GlLink, GlDropdown, GlDropdownItem } from '@gitlab/ui';
import { GlLink, GlDropdown, GlDropdownItem, GlSprintf } from '@gitlab/ui';
import { isEmpty } from 'lodash';
import Mousetrap from 'mousetrap';
import { s__ } from '~/locale';
import CiIcon from '~/vue_shared/components/ci_icon.vue';
import clipboardButton from '~/vue_shared/components/clipboard_button.vue';
import { clickCopyToClipboardButton } from '~/behaviors/copy_to_clipboard';
import { keysFor, MR_COPY_SOURCE_BRANCH_NAME } from '~/behaviors/shortcuts/keybindings';
export default {
components: {
......@@ -11,6 +15,7 @@ export default {
GlDropdown,
GlDropdownItem,
GlLink,
GlSprintf,
},
props: {
pipeline: {
......@@ -36,11 +41,43 @@ export default {
isMergeRequestPipeline() {
return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline);
},
pipelineInfo() {
if (!this.hasRef) {
return s__('Job|%{boldStart}Pipeline%{boldEnd} %{id}');
} else if (!this.isTriggeredByMergeRequest) {
return s__('Job|%{boldStart}Pipeline%{boldEnd} %{id} for %{ref}');
} else if (!this.isMergeRequestPipeline) {
return s__('Job|%{boldStart}Pipeline%{boldEnd} %{id} for %{mrId} with %{source}');
}
return s__(
'Job|%{boldStart}Pipeline%{boldEnd} %{id} for %{mrId} with %{source} into %{target}',
);
},
},
mounted() {
Mousetrap.bind(keysFor(MR_COPY_SOURCE_BRANCH_NAME), this.handleKeyboardCopy);
},
beforeDestroy() {
Mousetrap.unbind(keysFor(MR_COPY_SOURCE_BRANCH_NAME));
},
methods: {
onStageClick(stage) {
this.$emit('requestSidebarStageDropdown', stage);
},
handleKeyboardCopy() {
let button;
if (!this.hasRef) {
return;
} else if (!this.isTriggeredByMergeRequest) {
button = this.$refs['copy-source-ref-link'];
} else {
button = this.$refs['copy-source-branch-link'];
}
clickCopyToClipboardButton(button.$el);
},
},
};
</script>
......@@ -48,54 +85,72 @@ export default {
<div class="dropdown">
<div class="js-pipeline-info" data-testid="pipeline-info">
<ci-icon :status="pipeline.details.status" />
<span class="font-weight-bold">{{ s__('Job|Pipeline') }}</span>
<gl-link
:href="pipeline.path"
class="js-pipeline-path link-commit"
data-testid="pipeline-path"
data-qa-selector="pipeline_path"
>#{{ pipeline.id }}</gl-link
>
<template v-if="hasRef">
{{ s__('Job|for') }}
<template v-if="isTriggeredByMergeRequest">
<gl-sprintf :message="pipelineInfo">
<template #bold="{ content }">
<span class="font-weight-bold">{{ content }}</span>
</template>
<template #id>
<gl-link
:href="pipeline.path"
class="js-pipeline-path link-commit"
data-testid="pipeline-path"
data-qa-selector="pipeline_path"
>#{{ pipeline.id }}</gl-link
>
</template>
<template #mrId>
<gl-link
:href="pipeline.merge_request.path"
class="link-commit ref-name"
data-testid="mr-link"
>!{{ pipeline.merge_request.iid }}</gl-link
>
{{ s__('Job|with') }}
</template>
<template #ref>
<gl-link
:href="pipeline.ref.path"
class="link-commit ref-name"
data-testid="source-ref-link"
>{{ pipeline.ref.name }}</gl-link
><clipboard-button
ref="copy-source-ref-link"
:text="pipeline.ref.name"
:title="__('Copy reference')"
category="tertiary"
size="small"
data-testid="copy-source-ref-link"
/>
</template>
<template #source>
<gl-link
:href="pipeline.merge_request.source_branch_path"
class="link-commit ref-name"
data-testid="source-branch-link"
>{{ pipeline.merge_request.source_branch }}</gl-link
>
<template v-if="isMergeRequestPipeline">
{{ s__('Job|into') }}
<gl-link
:href="pipeline.merge_request.target_branch_path"
class="link-commit ref-name"
data-testid="target-branch-link"
>{{ pipeline.merge_request.target_branch }}</gl-link
>
</template>
><clipboard-button
ref="copy-source-branch-link"
:text="pipeline.merge_request.source_branch"
:title="__('Copy branch name')"
category="tertiary"
size="small"
data-testid="copy-source-branch-link"
/>
</template>
<template #target>
<gl-link
:href="pipeline.merge_request.target_branch_path"
class="link-commit ref-name"
data-testid="target-branch-link"
>{{ pipeline.merge_request.target_branch }}</gl-link
><clipboard-button
:text="pipeline.merge_request.target_branch"
:title="__('Copy branch name')"
category="tertiary"
size="small"
data-testid="copy-target-branch-link"
/>
</template>
<gl-link v-else :href="pipeline.ref.path" class="link-commit ref-name">{{
pipeline.ref.name
}}</gl-link
><clipboard-button
:text="pipeline.ref.name"
:title="__('Copy reference')"
category="tertiary"
size="small"
data-testid="copy-source-ref-link"
/>
</template>
</gl-sprintf>
</div>
<gl-dropdown :text="selectedStage" class="js-selected-stage gl-w-full gl-mt-3">
......
locale/gitlab.pot
View file @ 6aa1d555
......@@ -21028,6 +21028,18 @@ msgstr ""
msgid "Jobs|You're about to retry a job that failed because it attempted to deploy code that is older than the latest deployment. Retrying this job could result in overwriting the environment with the older source code."
msgstr ""
msgid "Job|%{boldStart}Pipeline%{boldEnd} %{id}"
msgstr ""
msgid "Job|%{boldStart}Pipeline%{boldEnd} %{id} for %{mrId} with %{source}"
msgstr ""
msgid "Job|%{boldStart}Pipeline%{boldEnd} %{id} for %{mrId} with %{source} into %{target}"
msgstr ""
msgid "Job|%{boldStart}Pipeline%{boldEnd} %{id} for %{ref}"
msgstr ""
msgid "Job|Are you sure you want to erase this job log and artifacts?"
msgstr ""
......@@ -21061,9 +21073,6 @@ msgstr ""
msgid "Job|Keep"
msgstr ""
msgid "Job|Pipeline"
msgstr ""
msgid "Job|Retry"
msgstr ""
......@@ -21106,21 +21115,12 @@ msgstr ""
msgid "Job|delayed"
msgstr ""
msgid "Job|for"
msgstr ""
msgid "Job|into"
msgstr ""
msgid "Job|manual"
msgstr ""
msgid "Job|triggered"
msgstr ""
msgid "Job|with"
msgstr ""
msgid "Join Zoom meeting"
msgstr ""
......
spec/frontend/jobs/components/stages_dropdown_spec.js
View file @ 6aa1d555
import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
import { GlDropdown, GlDropdownItem, GlLink, GlSprintf } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import { trimText } from 'helpers/text_helper';
import Mousetrap from 'mousetrap';
import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import StagesDropdown from '~/jobs/components/stages_dropdown.vue';
import CiIcon from '~/vue_shared/components/ci_icon.vue';
import * as copyToClipboard from '~/behaviors/copy_to_clipboard';
import {
mockPipelineWithoutRef,
mockPipelineWithoutMR,
mockPipelineWithAttachedMR,
mockPipelineDetached,
......@@ -18,20 +20,19 @@ describe('Stages Dropdown', () => {
const findStageItem = (index) => wrapper.findAllComponents(GlDropdownItem).at(index);
const findPipelineInfoText = () => wrapper.findByTestId('pipeline-info').text();
const findPipelinePath = () => wrapper.findByTestId('pipeline-path').attributes('href');
const findMRLinkPath = () => wrapper.findByTestId('mr-link').attributes('href');
const findCopySourceBranchBtn = () => wrapper.findByTestId('copy-source-ref-link');
const findSourceBranchLinkPath = () =>
wrapper.findByTestId('source-branch-link').attributes('href');
const findTargetBranchLinkPath = () =>
wrapper.findByTestId('target-branch-link').attributes('href');
const createComponent = (props) => {
wrapper = extendedWrapper(
shallowMount(StagesDropdown, {
propsData: {
stages: [],
selectedStage: 'deploy',
...props,
},
stubs: {
GlSprintf,
GlLink,
},
}),
);
};
......@@ -45,7 +46,6 @@ describe('Stages Dropdown', () => {
createComponent({
pipeline: mockPipelineWithoutMR,
stages: [{ name: 'build' }, { name: 'test' }],
selectedStage: 'deploy',
});
});
......@@ -53,10 +53,6 @@ describe('Stages Dropdown', () => {
expect(findStatus().exists()).toBe(true);
});
it('renders pipeline link', () => {
expect(findPipelinePath()).toBe('pipeline/28029444');
});
it('renders dropdown with stages', () => {
expect(findStageItem(0).text()).toBe('build');
});
......@@ -64,84 +60,133 @@ describe('Stages Dropdown', () => {
it('rendes selected stage', () => {
expect(findSelectedStageText()).toBe('deploy');
});
it(`renders the pipeline info text like "Pipeline #123 for source_branch"`, () => {
const expected = `Pipeline #${mockPipelineWithoutMR.id} for ${mockPipelineWithoutMR.ref.name}`;
const actual = trimText(findPipelineInfoText());
expect(actual).toBe(expected);
});
it(`renders the source ref copy button`, () => {
expect(findCopySourceBranchBtn().exists()).toBe(true);
});
});
describe('with an "attached" merge request pipeline', () => {
beforeEach(() => {
createComponent({
pipeline: mockPipelineWithAttachedMR,
stages: [],
selectedStage: 'deploy',
describe('pipelineInfo', () => {
const allElements = [
'pipeline-path',
'mr-link',
'source-ref-link',
'copy-source-ref-link',
'source-branch-link',
'copy-source-branch-link',
'target-branch-link',
'copy-target-branch-link',
];
describe.each([
[
'does not have a ref',
{
pipeline: mockPipelineWithoutRef,
text: `Pipeline #${mockPipelineWithoutRef.id}`,
foundElements: [
{ testId: 'pipeline-path', props: [{ href: mockPipelineWithoutRef.path }] },
],
},
],
[
'hasRef but not triggered by MR',
{
pipeline: mockPipelineWithoutMR,
text: `Pipeline #${mockPipelineWithoutMR.id} for ${mockPipelineWithoutMR.ref.name}`,
foundElements: [
{ testId: 'pipeline-path', props: [{ href: mockPipelineWithoutMR.path }] },
{ testId: 'source-ref-link', props: [{ href: mockPipelineWithoutMR.ref.path }] },
{ testId: 'copy-source-ref-link', props: [{ text: mockPipelineWithoutMR.ref.name }] },
],
},
],
[
'hasRef and MR but not MR pipeline',
{
pipeline: mockPipelineDetached,
text: `Pipeline #${mockPipelineDetached.id} for !${mockPipelineDetached.merge_request.iid} with ${mockPipelineDetached.merge_request.source_branch}`,
foundElements: [
{ testId: 'pipeline-path', props: [{ href: mockPipelineDetached.path }] },
{ testId: 'mr-link', props: [{ href: mockPipelineDetached.merge_request.path }] },
{
testId: 'source-branch-link',
props: [{ href: mockPipelineDetached.merge_request.source_branch_path }],
},
{
testId: 'copy-source-branch-link',
props: [{ text: mockPipelineDetached.merge_request.source_branch }],
},
],
},
],
[
'hasRef and MR and MR pipeline',
{
pipeline: mockPipelineWithAttachedMR,
text: `Pipeline #${mockPipelineWithAttachedMR.id} for !${mockPipelineWithAttachedMR.merge_request.iid} with ${mockPipelineWithAttachedMR.merge_request.source_branch} into ${mockPipelineWithAttachedMR.merge_request.target_branch}`,
foundElements: [
{ testId: 'pipeline-path', props: [{ href: mockPipelineWithAttachedMR.path }] },
{ testId: 'mr-link', props: [{ href: mockPipelineWithAttachedMR.merge_request.path }] },
{
testId: 'source-branch-link',
props: [{ href: mockPipelineWithAttachedMR.merge_request.source_branch_path }],
},
{
testId: 'copy-source-branch-link',
props: [{ text: mockPipelineWithAttachedMR.merge_request.source_branch }],
},
{
testId: 'target-branch-link',
props: [{ href: mockPipelineWithAttachedMR.merge_request.target_branch_path }],
},
{
testId: 'copy-target-branch-link',
props: [{ text: mockPipelineWithAttachedMR.merge_request.target_branch }],
},
],
},
],
])('%s', (_, { pipeline, text, foundElements }) => {
beforeEach(() => {
createComponent({
pipeline,
});
});
});
it(`renders the pipeline info text like "Pipeline #123 for !456 with source_branch into target_branch"`, () => {
const expected = `Pipeline #${mockPipelineWithAttachedMR.id} for !${mockPipelineWithAttachedMR.merge_request.iid} with ${mockPipelineWithAttachedMR.merge_request.source_branch} into ${mockPipelineWithAttachedMR.merge_request.target_branch}`;
const actual = trimText(findPipelineInfoText());
expect(actual).toBe(expected);
});
it(`renders the correct merge request link`, () => {
expect(findMRLinkPath()).toBe(mockPipelineWithAttachedMR.merge_request.path);
});
it(`renders the correct source branch link`, () => {
expect(findSourceBranchLinkPath()).toBe(
mockPipelineWithAttachedMR.merge_request.source_branch_path,
);
});
it(`renders the correct target branch link`, () => {
expect(findTargetBranchLinkPath()).toBe(
mockPipelineWithAttachedMR.merge_request.target_branch_path,
);
});
it(`renders the source ref copy button`, () => {
expect(findCopySourceBranchBtn().exists()).toBe(true);
});
});
describe('with a detached merge request pipeline', () => {
beforeEach(() => {
createComponent({
pipeline: mockPipelineDetached,
stages: [],
selectedStage: 'deploy',
it('should render the text', () => {
expect(findPipelineInfoText()).toMatchInterpolatedText(text);
});
});
it(`renders the pipeline info like "Pipeline #123 for !456 with source_branch"`, () => {
const expected = `Pipeline #${mockPipelineDetached.id} for !${mockPipelineDetached.merge_request.iid} with ${mockPipelineDetached.merge_request.source_branch}`;
const actual = trimText(findPipelineInfoText());
it('should find components with props', () => {
foundElements.forEach((element) => {
element.props.forEach((prop) => {
const key = Object.keys(prop)[0];
expect(wrapper.findByTestId(element.testId).attributes(key)).toBe(prop[key]);
});
});
});
expect(actual).toBe(expected);
it('should not find components', () => {
const foundTestIds = foundElements.map((element) => element.testId);
allElements
.filter((testId) => !foundTestIds.includes(testId))
.forEach((testId) => {
expect(wrapper.findByTestId(testId).exists()).toBe(false);
});
});
});
});
it(`renders the correct merge request link`, () => {
expect(findMRLinkPath()).toBe(mockPipelineDetached.merge_request.path);
});
describe('mousetrap', () => {
it.each([
['copy-source-ref-link', mockPipelineWithoutMR],
['copy-source-branch-link', mockPipelineWithAttachedMR],
])(
'calls clickCopyToClipboardButton with `%s` button when `b` is pressed',
(button, pipeline) => {
const copyToClipboardMock = jest.spyOn(copyToClipboard, 'clickCopyToClipboardButton');
createComponent({ pipeline });
it(`renders the correct source branch link`, () => {
expect(findSourceBranchLinkPath()).toBe(
mockPipelineDetached.merge_request.source_branch_path,
);
});
Mousetrap.trigger('b');
it(`renders the source ref copy button`, () => {
expect(findCopySourceBranchBtn().exists()).toBe(true);
});
expect(copyToClipboardMock).toHaveBeenCalledWith(wrapper.findByTestId(button).element);
},
);
});
});
spec/frontend/jobs/mock_data.js
View file @ 6aa1d555
......@@ -1214,6 +1214,11 @@ export const mockPipelineWithoutMR = {
},
};
export const mockPipelineWithoutRef = {
...mockPipelineWithoutMR,
ref: null,
};
export const mockPipelineWithAttachedMR = {
id: 28029444,
details: {
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7