From 7d88c2e56a50321fe81f3d7a0d27f708f46f9489 Mon Sep 17 00:00:00 2001
From: Clement Ho
Date: Tue, 20 Jun 2017 21:06:48 +0000
Subject: Revert "Merge branch 'winh-merge-request-related-issues' into
'master'"
This reverts merge request !11759
---
.../components/mr_widget_related_links.js | 41 +---
.../components/states/mr_widget_merged.js | 10 -
.../vue_merge_request_widget/mr_widget_options.js | 11 +-
.../stores/mr_widget_store.js | 13 --
.../vue_merge_request_widget/stores/state_maps.js | 13 ++
app/assets/stylesheets/pages/merge_requests.scss | 4 -
spec/features/merge_requests/closes_issues_spec.rb | 16 +-
.../components/mr_widget_related_links_spec.js | 246 +++++++--------------
.../vue_mr_widget/mr_widget_options_spec.js | 7 +-
.../vue_mr_widget/stores/mr_widget_store_spec.js | 12 -
10 files changed, 111 insertions(+), 262 deletions(-)
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.js
index 686cb38cbb1..205804670fa 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.js
@@ -1,63 +1,42 @@
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 and
+
- {{closingText}}
+ Closes {{issueLabel('closing')}}
.
{{issueLabel('mentioned')}}
- {{mentionedText}}
+ {{verbLabel('mentioned')}} mentioned but will not be closed.
-
+
`,
};
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js
index 9b8eed9016d..c7d32d18141 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js
@@ -1,9 +1,7 @@
/* 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.
-
-1;
},
shouldRenderPipelines() {
return Object.keys(this.mr.pipeline).length || this.mr.hasCI;
@@ -238,14 +238,9 @@ export default {
:is="componentName"
:mr="mr"
:service="service" />
-
+ :related-links="mr.relatedLinks" />
`,
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index ad73efb37e1..69bc1436284 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -1,18 +1,6 @@
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;
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
index dd939d98d0f..605dd3a1ff4 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
@@ -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,
};
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index c0bd045f1fc..2dc7f73a295 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -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;
diff --git a/spec/features/merge_requests/closes_issues_spec.rb b/spec/features/merge_requests/closes_issues_spec.rb
index 26444bb7a55..e627618042a 100644
--- a/spec/features/merge_requests/closes_issues_spec.rb
+++ b/spec/features/merge_requests/closes_issues_spec.rb
@@ -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
diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_related_links_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_related_links_spec.js
index 6a44c54cdee..f6e0c3dfb74 100644
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_related_links_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_related_links_spec.js
@@ -1,31 +1,20 @@
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: '#7',
- twoIssues: '#23 and #42',
- threeIssues: '#1, #2, and #3',
+ const data = {
+ relatedLinks: {
+ closing: '#23 and #42',
+ mentioned: '#7',
+ },
};
-
- 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: '#23 and #42',
- };
-
- 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: '#23 and #42',
+ },
+ });
+ 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: '#7',
- };
-
- 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: '#7',
+ },
+ });
- it('should have closing and mentioned issues at the same time', (done) => {
- vm.relatedLinks = {
- closing: '#7',
- mentioned: '#23 and #42',
- };
-
- 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: 'Assign yourself to these issues',
- };
-
- 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: '#7',
+ mentioned: '#23 and #42',
+ },
+ });
+ 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: '#7',
- mentioned: '#23 and #42',
- };
-
- 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: 'Assign yourself to these issues',
+ },
+ });
+
+ expect(vm.$el.innerText).toContain('Assign yourself to these issues');
});
});
});
diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
index 425dff89439..3a0c50b750f 100644
--- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
+++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
@@ -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();
});
});
diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
index 71285866302..56dd0198ae2 100644
--- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
+++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
@@ -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);
- });
});
});
--
cgit v1.2.1