Skip to content
Snippets Groups Projects
Commit 40eb8253 authored by Winnie Hellmann's avatar Winnie Hellmann
Browse files

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

This reverts commit 6ac777a7, reversing
changes made to 025cbc2a.
parent cf3cdd48
No related branches found
No related tags found
No related merge requests found
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
+ 10
− 31
View file @ 40eb8253
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
+ 0
− 10
View file @ 40eb8253
/* 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
+ 3
− 8
View file @ 40eb8253
@@ -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
+ 0
− 13
View file @ 40eb8253
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
+ 13
− 0
View file @ 40eb8253
@@ -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
+ 0
− 4
View file @ 40eb8253
@@ -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
+ 8
− 8
View file @ 40eb8253
@@ -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
+ 74
− 172
View file @ 40eb8253
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
+ 3
− 4
View file @ 40eb8253
@@ -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
+ 0
− 12
View file @ 40eb8253
@@ -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);
});
});
});
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