From 09babc575473eadc4ffabe3d469f95d4347cf76c Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 24 Jan 2018 13:24:12 +0000 Subject: [PATCH] Moves more mr widget components into vue files Adds i18n Adds better test coverage --- .../components/states/mr_widget_checking.js | 18 --- .../components/states/mr_widget_checking.vue | 23 +++ .../components/states/mr_widget_closed.js | 35 ---- .../components/states/mr_widget_closed.vue | 48 ++++++ .../components/states/mr_widget_conflicts.js | 47 ------ .../components/states/mr_widget_conflicts.vue | 61 +++++++ .../vue_merge_request_widget/dependencies.js | 6 +- .../unreleased/fl-mr-widget-refactor.yml | 5 + .../states/mr_widget_checking_spec.js | 36 +++-- .../states/mr_widget_closed_spec.js | 104 +++++------- .../states/mr_widget_conflicts_spec.js | 152 ++++++++---------- 11 files changed, 273 insertions(+), 262 deletions(-) delete mode 100644 app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.js create mode 100644 app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue delete mode 100644 app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js create mode 100644 app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.vue delete mode 100644 app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js create mode 100644 app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue create mode 100644 changelogs/unreleased/fl-mr-widget-refactor.yml diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.js deleted file mode 100644 index 09561694939..00000000000 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.js +++ /dev/null @@ -1,18 +0,0 @@ -import statusIcon from '../mr_widget_status_icon'; - -export default { - name: 'MRWidgetChecking', - components: { - statusIcon, - }, - template: ` -
- -
- - Checking ability to merge automatically - -
-
- `, -}; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue new file mode 100644 index 00000000000..04e1766b8c7 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue @@ -0,0 +1,23 @@ + + diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js deleted file mode 100644 index dc19b20aa11..00000000000 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js +++ /dev/null @@ -1,35 +0,0 @@ -import mrWidgetAuthorTime from '../../components/mr_widget_author_time'; -import statusIcon from '../mr_widget_status_icon'; - -export default { - name: 'MRWidgetClosed', - props: { - mr: { type: Object, required: true }, - }, - components: { - 'mr-widget-author-and-time': mrWidgetAuthorTime, - statusIcon, - }, - template: ` -
- -
- -
-

- The changes were not merged into - - {{mr.targetBranch}} -

-
-
-
- `, -}; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.vue new file mode 100644 index 00000000000..8c0ce43c76c --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.vue @@ -0,0 +1,48 @@ + + diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js deleted file mode 100644 index 7a887bacfa7..00000000000 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js +++ /dev/null @@ -1,47 +0,0 @@ -import statusIcon from '../mr_widget_status_icon'; - -export default { - name: 'MRWidgetConflicts', - props: { - mr: { type: Object, required: true }, - }, - components: { - statusIcon, - }, - template: ` -
- -
- - Fast-forward merge is not possible. - To merge this request, first rebase locally. - - -
-
- `, -}; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue new file mode 100644 index 00000000000..13b07f82330 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue @@ -0,0 +1,61 @@ + + diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js index 2dd3b2c2f98..5e8e251428a 100644 --- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js +++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js @@ -18,11 +18,11 @@ export { default as WidgetDeployment } from './components/mr_widget_deployment'; export { default as WidgetRelatedLinks } from './components/mr_widget_related_links'; export { default as MergedState } from './components/states/mr_widget_merged'; export { default as FailedToMerge } from './components/states/mr_widget_failed_to_merge'; -export { default as ClosedState } from './components/states/mr_widget_closed'; +export { default as ClosedState } from './components/states/mr_widget_closed.vue'; export { default as MergingState } from './components/states/mr_widget_merging'; export { default as WipState } from './components/states/mr_widget_wip'; export { default as ArchivedState } from './components/states/mr_widget_archived.vue'; -export { default as ConflictsState } from './components/states/mr_widget_conflicts'; +export { default as ConflictsState } from './components/states/mr_widget_conflicts.vue'; export { default as NothingToMergeState } from './components/states/mr_widget_nothing_to_merge'; export { default as MissingBranchState } from './components/states/mr_widget_missing_branch'; export { default as NotAllowedState } from './components/states/mr_widget_not_allowed'; @@ -34,7 +34,7 @@ export { default as PipelineFailedState } from './components/states/mr_widget_pi export { default as MergeWhenPipelineSucceedsState } from './components/states/mr_widget_merge_when_pipeline_succeeds'; export { default as RebaseState } from './components/states/mr_widget_rebase.vue'; export { default as AutoMergeFailed } from './components/states/mr_widget_auto_merge_failed.vue'; -export { default as CheckingState } from './components/states/mr_widget_checking'; +export { default as CheckingState } from './components/states/mr_widget_checking.vue'; export { default as MRWidgetStore } from './stores/mr_widget_store'; export { default as MRWidgetService } from './services/mr_widget_service'; export { default as eventHub } from './event_hub'; diff --git a/changelogs/unreleased/fl-mr-widget-refactor.yml b/changelogs/unreleased/fl-mr-widget-refactor.yml new file mode 100644 index 00000000000..d59cca68409 --- /dev/null +++ b/changelogs/unreleased/fl-mr-widget-refactor.yml @@ -0,0 +1,5 @@ +--- +title: Refactors mr widget components into vue files and adds i18n +merge_request: +author: +type: other diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js index 6b7aa935ad3..658cadddb81 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js @@ -1,19 +1,29 @@ import Vue from 'vue'; -import checkingComponent from '~/vue_merge_request_widget/components/states/mr_widget_checking'; +import checkingComponent from '~/vue_merge_request_widget/components/states/mr_widget_checking.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; describe('MRWidgetChecking', () => { - describe('template', () => { - it('should have correct elements', () => { - const Component = Vue.extend(checkingComponent); - const el = new Component({ - el: document.createElement('div'), - }).$el; + let Component; + let vm; - expect(el.classList.contains('mr-widget-body')).toBeTruthy(); - expect(el.querySelector('button').classList.contains('btn-success')).toBeTruthy(); - expect(el.querySelector('button').disabled).toBeTruthy(); - expect(el.innerText).toContain('Checking ability to merge automatically'); - expect(el.querySelector('i')).toBeDefined(); - }); + beforeEach(() => { + Component = Vue.extend(checkingComponent); + vm = mountComponent(Component); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders disabled button', () => { + expect(vm.$el.querySelector('button').getAttribute('disabled')).toEqual('disabled'); + }); + + it('renders loading icon', () => { + expect(vm.$el.querySelector('.mr-widget-icon i').classList).toContain('fa-spinner'); + }); + + it('renders information about merging', () => { + expect(vm.$el.querySelector('.media-body').textContent.trim()).toEqual('Checking ability to merge automatically'); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js index 1bf97bbf093..51a34739ee9 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js @@ -1,74 +1,58 @@ import Vue from 'vue'; -import closedComponent from '~/vue_merge_request_widget/components/states/mr_widget_closed'; - -const mr = { - targetBranch: 'good-branch', - targetBranchPath: '/good-branch', - metrics: { - mergedBy: {}, - mergedAt: 'mergedUpdatedAt', - closedBy: { - name: 'Fatih Acet', - username: 'fatihacet', - }, - closedAt: 'closedEventUpdatedAt', - readableMergedAt: '', - readableClosedAt: '', - }, - updatedAt: 'mrUpdatedAt', - closedAt: '1 day ago', -}; - -const createComponent = () => { - const Component = Vue.extend(closedComponent); - - return new Component({ - el: document.createElement('div'), - propsData: { mr }, - }); -}; +import closedComponent from '~/vue_merge_request_widget/components/states/mr_widget_closed.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; describe('MRWidgetClosed', () => { - describe('props', () => { - it('should have props', () => { - const mrProp = closedComponent.props.mr; + let vm; - expect(mrProp.type instanceof Object).toBeTruthy(); - expect(mrProp.required).toBeTruthy(); - }); + beforeEach(() => { + const Component = Vue.extend(closedComponent); + vm = mountComponent(Component, { mr: { + metrics: { + mergedBy: {}, + closedBy: { + name: 'Administrator', + username: 'root', + webUrl: 'http://localhost:3000/root', + avatarUrl: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + }, + mergedAt: 'Jan 24, 2018 1:02pm GMT+0000', + closedAt: 'Jan 24, 2018 1:02pm GMT+0000', + readableMergedAt: '', + readableClosedAt: 'less than a minute ago', + }, + targetBranchPath: '/twitter/flight/commits/so_long_jquery', + targetBranch: 'so_long_jquery', + } }); }); - describe('components', () => { - it('should have components added', () => { - expect(closedComponent.components['mr-widget-author-and-time']).toBeDefined(); - }); + afterEach(() => { + vm.$destroy(); }); - describe('template', () => { - let vm; - let el; + it('renders warning icon', () => { + expect(vm.$el.querySelector('.js-ci-status-icon-warning')).not.toBeNull(); + }); - beforeEach(() => { - vm = createComponent(); - el = vm.$el; - }); + it('renders closed by information with author and time', () => { + expect( + vm.$el.querySelector('.js-mr-widget-author').textContent.trim().replace(/\s\s+/g, ' '), + ).toContain( + 'Closed by Administrator less than a minute ago', + ); + }); - afterEach(() => { - vm.$destroy(); - }); + it('links to the user that closed the MR', () => { + expect(vm.$el.querySelector('.author-link').getAttribute('href')).toEqual('http://localhost:3000/root'); + }); - it('should have correct elements', () => { - expect(el.querySelector('h4').textContent).toContain('Closed by'); - expect(el.querySelector('h4').textContent).toContain(mr.metrics.closedBy.name); - expect(el.textContent).toContain('The changes were not merged into'); - expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath); - expect(el.querySelector('.label-branch').textContent).toContain(mr.targetBranch); - }); + it('renders information about the changes not being merged', () => { + expect( + vm.$el.querySelector('.mr-info-list').textContent.trim().replace(/\s\s+/g, ' '), + ).toContain('The changes were not merged into so_long_jquery'); + }); - it('should use closedEvent updatedAt as tooltip title', () => { - expect( - el.querySelector('time').getAttribute('title'), - ).toBe('closedEventUpdatedAt'); - }); + it('renders link for target branch', () => { + expect(vm.$el.querySelector('.label-branch').getAttribute('href')).toEqual('/twitter/flight/commits/so_long_jquery'); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js index 5d4c7ec09dc..a7d69fdcdb9 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js @@ -1,105 +1,85 @@ import Vue from 'vue'; -import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts'; +import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts.vue'; import mountComponent from '../../../helpers/vue_mount_component_helper'; -const ConflictsComponent = Vue.extend(conflictsComponent); -const path = '/conflicts'; - describe('MRWidgetConflicts', () => { - describe('props', () => { - it('should have props', () => { - const { mr } = conflictsComponent.props; + let Component; + let vm; + const path = '/conflicts'; - expect(mr.type instanceof Object).toBeTruthy(); - expect(mr.required).toBeTruthy(); + beforeEach(() => { + Component = Vue.extend(conflictsComponent); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('when allowed to merge', () => { + beforeEach(() => { + vm = mountComponent(Component, { + mr: { + canMerge: true, + conflictResolutionPath: path, + }, + }); + }); + + it('should tell you about conflicts without bothering other people', () => { + expect(vm.$el.textContent).toContain('There are merge conflicts'); + expect(vm.$el.textContent).not.toContain('ask someone with write access'); + }); + + it('should allow you to resolve the conflicts', () => { + const resolveButton = vm.$el.querySelector('.js-resolve-conflicts-button'); + + expect(resolveButton.textContent).toContain('Resolve conflicts'); + expect(resolveButton.getAttribute('href')).toEqual(path); + }); + + it('should have merge buttons', () => { + const mergeButton = vm.$el.querySelector('.js-disabled-merge-button'); + const mergeLocallyButton = vm.$el.querySelector('.js-merge-locally-button'); + + expect(mergeButton.textContent).toContain('Merge'); + expect(mergeButton.disabled).toBeTruthy(); + expect(mergeButton.classList.contains('btn-success')).toEqual(true); + expect(mergeLocallyButton.textContent).toContain('Merge locally'); }); }); - describe('template', () => { - describe('when allowed to merge', () => { - let vm; - - beforeEach(() => { - vm = mountComponent(ConflictsComponent, { - mr: { - canMerge: true, - conflictResolutionPath: path, - }, - }); - }); - - afterEach(() => { - vm.$destroy(); - }); - - it('should tell you about conflicts without bothering other people', () => { - expect(vm.$el.textContent).toContain('There are merge conflicts'); - expect(vm.$el.textContent).not.toContain('ask someone with write access'); - }); - - it('should allow you to resolve the conflicts', () => { - const resolveButton = vm.$el.querySelector('.js-resolve-conflicts-button'); - - expect(resolveButton.textContent).toContain('Resolve conflicts'); - expect(resolveButton.getAttribute('href')).toEqual(path); - }); - - it('should have merge buttons', () => { - const mergeButton = vm.$el.querySelector('.js-disabled-merge-button'); - const mergeLocallyButton = vm.$el.querySelector('.js-merge-locally-button'); - - expect(mergeButton.textContent).toContain('Merge'); - expect(mergeButton.disabled).toBeTruthy(); - expect(mergeButton.classList.contains('btn-success')).toEqual(true); - expect(mergeLocallyButton.textContent).toContain('Merge locally'); + describe('when user does not have permission to merge', () => { + beforeEach(() => { + vm = mountComponent(Component, { + mr: { + canMerge: false, + }, }); }); - describe('when user does not have permission to merge', () => { - let vm; + it('should show proper message', () => { + expect(vm.$el.textContent.trim().replace(/\s\s+/g, ' ')).toContain('ask someone with write access'); + }); - beforeEach(() => { - vm = mountComponent(ConflictsComponent, { - mr: { - canMerge: false, - }, - }); - }); + it('should not have action buttons', () => { + expect(vm.$el.querySelector('.js-disabled-merge-button')).toBeDefined(); + expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toBeNull(); + expect(vm.$el.querySelector('.js-merge-locally-button')).toBeNull(); + }); + }); - afterEach(() => { - vm.$destroy(); - }); - - it('should show proper message', () => { - expect(vm.$el.textContent).toContain('ask someone with write access'); - }); - - it('should not have action buttons', () => { - expect(vm.$el.querySelector('.js-disabled-merge-button')).toBeDefined(); - expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toBeNull(); - expect(vm.$el.querySelector('.js-merge-locally-button')).toBeNull(); + describe('when fast-forward or semi-linear merge enabled', () => { + beforeEach(() => { + vm = mountComponent(Component, { + mr: { + shouldBeRebased: true, + }, }); }); - describe('when fast-forward or semi-linear merge enabled', () => { - let vm; - - beforeEach(() => { - vm = mountComponent(ConflictsComponent, { - mr: { - shouldBeRebased: true, - }, - }); - }); - - afterEach(() => { - vm.$destroy(); - }); - - it('should tell you to rebase locally', () => { - expect(vm.$el.textContent).toContain('Fast-forward merge is not possible.'); - expect(vm.$el.textContent).toContain('To merge this request, first rebase locally'); - }); + it('should tell you to rebase locally', () => { + expect(vm.$el.textContent.trim().replace(/\s\s+/g, ' ')).toContain('Fast-forward merge is not possible.'); + expect(vm.$el.textContent.trim().replace(/\s\s+/g, ' ')).toContain('To merge this request, first rebase locally'); }); }); });