Skip to content

G
gitlab-ce
Commit 1e42253c authored by Jacob Schatz's avatar Jacob Schatz
Browse files
Options

Merge branch 'winh-revert-6ac777a7' into 'master' 

Revert "Merge branch 'winh-merge-request-related-issues' into 'master'"

See merge request !12354
parents 6431118d 40eb8253
Hide whitespace changes
Inline Side-by-side
Showing with 111 additions and 262 deletions
app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.js
View file @ 1e42253c
export default {
name: 'MRWidgetRelatedLinks',
props: {
isMerged: { type: Boolean, required: true },
relatedLinks: { type: Object, required: true },
},
computed: {
// TODO: the following should be handled by i18n
closingText() {
if (this.isMerged) {
return `Closed ${this.issueLabel('closing')}`;
}
return `Closes ${this.issueLabel('closing')}`;
},
hasLinks() {
const { closing, mentioned, assignToMe } = this.relatedLinks;
return closing || mentioned || assignToMe;
},
// TODO: the following should be handled by i18n
mentionedText() {
if (this.isMerged) {
if (this.hasMultipleIssues(this.relatedLinks.mentioned)) {
return 'are mentioned but were not closed';
}
return 'is mentioned but was not closed';
}
if (this.hasMultipleIssues(this.relatedLinks.mentioned)) {
return 'are mentioned but will not be closed';
}
return 'is mentioned but will not be closed';
},
},
methods: {
hasMultipleIssues(text) {
return /<\/a>,? and <a/.test(text);
return !text ? false : text.match(/<\/a> and <a/);
},
// TODO: the following should be handled by i18n
issueLabel(field) {
return this.hasMultipleIssues(this.relatedLinks[field]) ? 'issues' : 'issue';
},
verbLabel(field) {
return this.hasMultipleIssues(this.relatedLinks[field]) ? 'are' : 'is';
},
},
template: `
<div v-if="hasLinks">
<section
v-if="hasLinks"
class="mr-info-list mr-links">
<div class="legend"></div>
<p v-if="relatedLinks.closing">
{{closingText}}
Closes {{issueLabel('closing')}}
<span v-html="relatedLinks.closing"></span>.
</p>
<p v-if="relatedLinks.mentioned">
<span class="capitalize">{{issueLabel('mentioned')}}</span>
<span v-html="relatedLinks.mentioned"></span>
{{mentionedText}}
{{verbLabel('mentioned')}} mentioned but will not be closed.
</p>
<p v-if="relatedLinks.assignToMe">
<span v-html="relatedLinks.assignToMe"></span>
</p>
</div>
</section>
`,
};
app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js
View file @ 1e42253c
/* global Flash */
import mrWidgetAuthorTime from '../../components/mr_widget_author_time';
import mrWidgetRelatedLinks from '../../components/mr_widget_related_links';
import eventHub from '../../event_hub';
import '../../../flash';
export default {
name: 'MRWidgetMerged',
......@@ -13,7 +11,6 @@ export default {
},
components: {
'mr-widget-author-and-time': mrWidgetAuthorTime,
'mr-widget-related-links': mrWidgetRelatedLinks,
},
data() {
return {
......@@ -21,9 +18,6 @@ export default {
};
},
computed: {
shouldRenderRelatedLinks() {
return this.mr.relatedLinks && this.mr.isMerged;
},
shouldShowRemoveSourceBranch() {
const { sourceBranchRemoved, isRemovingSourceBranch, canRemoveSourceBranch } = this.mr;
......@@ -92,10 +86,6 @@ export default {
aria-hidden="true" />
The source branch is being removed.
</p>
<mr-widget-related-links
v-if="shouldRenderRelatedLinks"
:is-merged="mr.isMerged()"
:related-links="mr.relatedLinks" />
</section>
<div
v-if="shouldShowMergedButtons"
......
app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
View file @ 1e42253c
......@@ -48,7 +48,7 @@ export default {
return stateMaps.stateToComponentMap[this.mr.state];
},
shouldRenderMergeHelp() {
return !this.mr.isMerged;
return stateMaps.statesToShowHelpWidget.indexOf(this.mr.state) > -1;
},
shouldRenderPipelines() {
return Object.keys(this.mr.pipeline).length || this.mr.hasCI;
......@@ -238,14 +238,9 @@ export default {
:is="componentName"
:mr="mr"
:service="service" />
<section
<mr-widget-related-links
v-if="shouldRenderRelatedLinks"
class="mr-info-list mr-links">
<div class="legend"></div>
<mr-widget-related-links
:is-merged="mr.isMerged"
:related-links="mr.relatedLinks" />
</section>
:related-links="mr.relatedLinks" />
<mr-widget-merge-help v-if="shouldRenderMergeHelp" />
</div>
`,
......
app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
View file @ 1e42253c
import Timeago from 'timeago.js';
import { getStateKey } from '../dependencies';
const unmergedStates = [
'locked',
'conflicts',
'workInProgress',
'readyToMerge',
'checking',
'unresolvedDiscussions',
'pipelineFailed',
'pipelineBlocked',
'autoMergeFailed',
];
export default class MergeRequestStore {
constructor(data) {
......@@ -77,7 +65,6 @@ export default class MergeRequestStore {
this.mergeActionsContentPath = data.commit_change_content_path;
this.isRemovingSourceBranch = this.isRemovingSourceBranch || false;
this.isOpen = data.state === 'opened' || data.state === 'reopened' || false;
this.isMerged = unmergedStates.indexOf(data.state) === -1;
this.hasMergeableDiscussionsState = data.mergeable_discussions_state === false;
this.canRemoveSourceBranch = currentUser.can_remove_source_branch || false;
this.canMerge = !!data.merge_path;
......
app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
View file @ 1e42253c
......@@ -19,6 +19,19 @@ const stateToComponentMap = {
shaMismatch: 'mr-widget-sha-mismatch',
};
const statesToShowHelpWidget = [
'locked',
'conflicts',
'workInProgress',
'readyToMerge',
'checking',
'unresolvedDiscussions',
'pipelineFailed',
'pipelineBlocked',
'autoMergeFailed',
];
export default {
stateToComponentMap,
statesToShowHelpWidget,
};
app/assets/stylesheets/pages/merge_requests.scss
View file @ 1e42253c
......@@ -372,10 +372,6 @@
margin-left: 12px;
}
&.mr-state-locked + .mr-info-list.mr-links {
margin-top: -16px;
}
&.empty-state {
.artwork {
margin-bottom: $gl-padding;
......
spec/features/merge_requests/closes_issues_spec.rb
View file @ 1e42253c
......@@ -36,7 +36,7 @@ feature 'Merge Request closing issues message', feature: true, js: true do
let(:merge_request_description) { "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" }
it 'does not display closing issue message' do
expect(page).to have_content("Closed issues #{issue_1.to_reference} and #{issue_2.to_reference}")
expect(page).to have_content("Closes issues #{issue_1.to_reference} and #{issue_2.to_reference}")
end
end
......@@ -44,7 +44,7 @@ feature 'Merge Request closing issues message', feature: true, js: true do
let(:merge_request_description) { "Description\n\nRefers to #{issue_1.to_reference} and #{issue_2.to_reference}" }
it 'does not display closing issue message' do
expect(page).to have_content("Issues #{issue_1.to_reference} and #{issue_2.to_reference} are mentioned but were not closed")
expect(page).to have_content("Issues #{issue_1.to_reference} and #{issue_2.to_reference} are mentioned but will not be closed.")
end
end
......@@ -52,8 +52,8 @@ feature 'Merge Request closing issues message', feature: true, js: true do
let(:merge_request_title) { "closes #{issue_1.to_reference}\n\n refers to #{issue_2.to_reference}" }
it 'does not display closing issue message' do
expect(page).to have_content("Closed issue #{issue_1.to_reference}")
expect(page).to have_content("Issue #{issue_2.to_reference} is mentioned but was not closed")
expect(page).to have_content("Closes issue #{issue_1.to_reference}.")
expect(page).to have_content("Issue #{issue_2.to_reference} is mentioned but will not be closed.")
end
end
......@@ -61,7 +61,7 @@ feature 'Merge Request closing issues message', feature: true, js: true do
let(:merge_request_title) { "closing #{issue_1.to_reference}, #{issue_2.to_reference}" }
it 'does not display closing issue message' do
expect(page).to have_content("Closed issues #{issue_1.to_reference} and #{issue_2.to_reference}")
expect(page).to have_content("Closes issues #{issue_1.to_reference} and #{issue_2.to_reference}")
end
end
......@@ -69,7 +69,7 @@ feature 'Merge Request closing issues message', feature: true, js: true do
let(:merge_request_title) { "Refers to #{issue_1.to_reference} and #{issue_2.to_reference}" }
it 'does not display closing issue message' do
expect(page).to have_content("Issues #{issue_1.to_reference} and #{issue_2.to_reference} are mentioned but were not closed")
expect(page).to have_content("Issues #{issue_1.to_reference} and #{issue_2.to_reference} are mentioned but will not be closed.")
end
end
......@@ -77,8 +77,8 @@ feature 'Merge Request closing issues message', feature: true, js: true do
let(:merge_request_title) { "closes #{issue_1.to_reference}\n\n refers to #{issue_2.to_reference}" }
it 'does not display closing issue message' do
expect(page).to have_content("Closed issue #{issue_1.to_reference}")
expect(page).to have_content("Issue #{issue_2.to_reference} is mentioned but was not closed")
expect(page).to have_content("Closes issue #{issue_1.to_reference}. Issue #{issue_2.to_reference} is mentioned but will not be closed.")
expect(page).to have_content("Issue #{issue_2.to_reference} is mentioned but will not be closed.")
end
end
end
spec/javascripts/vue_mr_widget/components/mr_widget_related_links_spec.js
View file @ 1e42253c
import Vue from 'vue';
import MRWidgetRelatedLinks from '~/vue_merge_request_widget/components/mr_widget_related_links';
import relatedLinksComponent from '~/vue_merge_request_widget/components/mr_widget_related_links';
describe('MRWidgetRelatedLinks', () => {
let vm;
beforeEach(() => {
const Component = Vue.extend(MRWidgetRelatedLinks);
vm = new Component({
el: document.createElement('div'),
propsData: {
isMerged: false,
relatedLinks: {},
},
});
});
const createComponent = (data) => {
const Component = Vue.extend(relatedLinksComponent);
afterEach(() => {
vm.$destroy();
return new Component({
el: document.createElement('div'),
propsData: data,
});
};
describe('MRWidgetRelatedLinks', () => {
describe('props', () => {
it('should have props', () => {
const { isMerged, relatedLinks } = MRWidgetRelatedLinks.props;
const { relatedLinks } = relatedLinksComponent.props;
expect(isMerged).toBeDefined();
expect(isMerged.type).toBe(Boolean);
expect(isMerged.required).toBeTruthy();
expect(relatedLinks).toBeDefined();
expect(relatedLinks.type instanceof Object).toBeTruthy();
expect(relatedLinks.required).toBeTruthy();
......@@ -33,38 +22,16 @@ describe('MRWidgetRelatedLinks', () => {
});
describe('computed', () => {
describe('closingText', () => {
const dummyIssueLabel = 'dummy label';
beforeEach(() => {
spyOn(vm, 'issueLabel').and.returnValue(dummyIssueLabel);
});
it('outputs text for closing issues', () => {
vm.isMerged = false;
const text = vm.closingText;
expect(text).toBe(`Closes ${dummyIssueLabel}`);
});
it('outputs text for closed issues', () => {
vm.isMerged = true;
const text = vm.closingText;
expect(text).toBe(`Closed ${dummyIssueLabel}`);
});
});
describe('hasLinks', () => {
it('should return correct value when we have links reference', () => {
vm.relatedLinks = {
closing: '/foo',
mentioned: '/foo',
assignToMe: '/foo',
const data = {
relatedLinks: {
closing: '/foo',
mentioned: '/foo',
assignToMe: '/foo',
},
};
const vm = createComponent(data);
expect(vm.hasLinks).toBeTruthy();
vm.relatedLinks.closing = null;
......@@ -77,160 +44,95 @@ describe('MRWidgetRelatedLinks', () => {
expect(vm.hasLinks).toBeFalsy();
});
});
describe('mentionedText', () => {
it('outputs text for one mentioned issue before merging', () => {
vm.isMerged = false;
spyOn(vm, 'hasMultipleIssues').and.returnValue(false);
const text = vm.mentionedText;
expect(text).toBe('is mentioned but will not be closed');
});
it('outputs text for one mentioned issue after merging', () => {
vm.isMerged = true;
spyOn(vm, 'hasMultipleIssues').and.returnValue(false);
const text = vm.mentionedText;
expect(text).toBe('is mentioned but was not closed');
});
it('outputs text for multiple mentioned issue before merging', () => {
vm.isMerged = false;
spyOn(vm, 'hasMultipleIssues').and.returnValue(true);
const text = vm.mentionedText;
expect(text).toBe('are mentioned but will not be closed');
});
it('outputs text for multiple mentioned issue after merging', () => {
vm.isMerged = true;
spyOn(vm, 'hasMultipleIssues').and.returnValue(true);
const text = vm.mentionedText;
expect(text).toBe('are mentioned but were not closed');
});
});
});
describe('methods', () => {
const relatedLinks = {
oneIssue: '<a href="#">#7</a>',
twoIssues: '<a href="#">#23</a> and <a>#42</a>',
threeIssues: '<a href="#">#1</a>, <a>#2</a>, and <a>#3</a>',
const data = {
relatedLinks: {
closing: '<a href="#">#23</a> and <a>#42</a>',
mentioned: '<a href="#">#7</a>',
},
};
beforeEach(() => {
vm.relatedLinks = relatedLinks;
});
const vm = createComponent(data);
describe('hasMultipleIssues', () => {
it('should return true if the given text has multiple issues', () => {
expect(vm.hasMultipleIssues(relatedLinks.twoIssues)).toBeTruthy();
expect(vm.hasMultipleIssues(relatedLinks.threeIssues)).toBeTruthy();
expect(vm.hasMultipleIssues(data.relatedLinks.closing)).toBeTruthy();
});
it('should return false if the given text has one issue', () => {
expect(vm.hasMultipleIssues(relatedLinks.oneIssue)).toBeFalsy();
expect(vm.hasMultipleIssues(data.relatedLinks.mentioned)).toBeFalsy();
});
});
describe('issueLabel', () => {
it('should return true if the given text has multiple issues', () => {
expect(vm.issueLabel('twoIssues')).toEqual('issues');
expect(vm.issueLabel('threeIssues')).toEqual('issues');
expect(vm.issueLabel('closing')).toEqual('issues');
});
it('should return false if the given text has one issue', () => {
expect(vm.issueLabel('mentioned')).toEqual('issue');
});
});
describe('verbLabel', () => {
it('should return true if the given text has multiple issues', () => {
expect(vm.verbLabel('closing')).toEqual('are');
});
it('should return false if the given text has one issue', () => {
expect(vm.issueLabel('oneIssue')).toEqual('issue');
expect(vm.verbLabel('mentioned')).toEqual('is');
});
});
});
describe('template', () => {
it('should have only have closing issues text', (done) => {
vm.relatedLinks = {
closing: '<a href="#">#23</a> and <a>#42</a>',
};
Vue.nextTick()
.then(() => {
const content = vm.$el.textContent.replace(/\n(\s)+/g, ' ').trim();
it('should have only have closing issues text', () => {
const vm = createComponent({
relatedLinks: {
closing: '<a href="#">#23</a> and <a>#42</a>',
},
});
const content = vm.$el.textContent.replace(/\n(\s)+/g, ' ').trim();
expect(content).toContain('Closes issues #23 and #42');
expect(content).not.toContain('mentioned');
})
.then(done)
.catch(done.fail);
expect(content).toContain('Closes issues #23 and #42');
expect(content).not.toContain('mentioned');
});
it('should have only have mentioned issues text', (done) => {
vm.relatedLinks = {
mentioned: '<a href="#">#7</a>',
};
Vue.nextTick()
.then(() => {
expect(vm.$el.innerText).toContain('issue #7');
expect(vm.$el.innerText).toContain('is mentioned but will not be closed');
expect(vm.$el.innerText).not.toContain('Closes');
})
.then(done)
.catch(done.fail);
});
it('should have only have mentioned issues text', () => {
const vm = createComponent({
relatedLinks: {
mentioned: '<a href="#">#7</a>',
},
});
it('should have closing and mentioned issues at the same time', (done) => {
vm.relatedLinks = {
closing: '<a href="#">#7</a>',
mentioned: '<a href="#">#23</a> and <a>#42</a>',
};
Vue.nextTick()
.then(() => {
const content = vm.$el.textContent.replace(/\n(\s)+/g, ' ').trim();
expect(content).toContain('Closes issue #7.');
expect(content).toContain('issues #23 and #42');
expect(content).toContain('are mentioned but will not be closed');
})
.then(done)
.catch(done.fail);
expect(vm.$el.innerText).toContain('issue #7');
expect(vm.$el.innerText).toContain('is mentioned but will not be closed.');
expect(vm.$el.innerText).not.toContain('Closes');
});
it('should have assing issues link', (done) => {
vm.relatedLinks = {
assignToMe: '<a href="#">Assign yourself to these issues</a>',
};
Vue.nextTick()
.then(() => {
expect(vm.$el.innerText).toContain('Assign yourself to these issues');
})
.then(done)
.catch(done.fail);
it('should have closing and mentioned issues at the same time', () => {
const vm = createComponent({
relatedLinks: {
closing: '<a href="#">#7</a>',
mentioned: '<a href="#">#23</a> and <a>#42</a>',
},
});
const content = vm.$el.textContent.replace(/\n(\s)+/g, ' ').trim();
expect(content).toContain('Closes issue #7.');
expect(content).toContain('issues #23 and #42');
expect(content).toContain('are mentioned but will not be closed.');
});
it('should use different wording after merging', (done) => {
vm.isMerged = true;
vm.relatedLinks = {
closing: '<a href="#">#7</a>',
mentioned: '<a href="#">#23</a> and <a>#42</a>',
};
Vue.nextTick()
.then(() => {
const content = vm.$el.textContent.replace(/\n(\s)+/g, ' ').trim();
expect(content).toContain('Closed issue #7.');
expect(content).toContain('issues #23 and #42');
expect(content).toContain('are mentioned but were not closed');
})
.then(done)
.catch(done.fail);
it('should have assing issues link', () => {
const vm = createComponent({
relatedLinks: {
assignToMe: '<a href="#">Assign yourself to these issues</a>',
},
});
expect(vm.$el.innerText).toContain('Assign yourself to these issues');
});
});
});
spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
View file @ 1e42253c
......@@ -48,13 +48,12 @@ describe('mrWidgetOptions', () => {
});
describe('shouldRenderMergeHelp', () => {
it('should return false after merging', () => {
vm.mr.isMerged = true;
it('should return false for the initial merged state', () => {
expect(vm.shouldRenderMergeHelp).toBeFalsy();
});
it('should return true before merging', () => {
vm.mr.isMerged = false;
it('should return true for a state which requires help widget', () => {
vm.mr.state = 'conflicts';
expect(vm.shouldRenderMergeHelp).toBeTruthy();
});
});
......
spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
View file @ 1e42253c
......@@ -18,17 +18,5 @@ describe('MergeRequestStore', () => {
store.setData({ ...mockData, work_in_progress: !mockData.work_in_progress });
expect(store.hasSHAChanged).toBe(false);
});
it('sets isMerged to true for merged state', () => {
store.setData({ ...mockData, state: 'merged' });
expect(store.isMerged).toBe(true);
});
it('sets isMerged to false for readyToMerge state', () => {
store.setData({ ...mockData, state: 'readyToMerge' });
expect(store.isMerged).toBe(false);
});
});
});
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