From 2a9b18309e6145e323e23a9afe32e1f87e347b31 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 9 Apr 2018 19:04:22 +0100 Subject: [PATCH] Replace vue resources with axios for pipelines table --- .../commit/pipelines/pipelines_table.vue | 26 ++- .../pipelines/components/pipelines.vue | 64 +++---- .../pipelines/services/pipelines_service.js | 37 ++-- .../unreleased/40487-axios-pipelines.yml | 4 + .../commit/pipelines/pipelines_spec.js | 165 +++++++----------- spec/javascripts/pipelines/pipelines_spec.js | 143 ++++++--------- 6 files changed, 172 insertions(+), 267 deletions(-) create mode 100644 changelogs/unreleased/40487-axios-pipelines.yml diff --git a/app/assets/javascripts/commit/pipelines/pipelines_table.vue b/app/assets/javascripts/commit/pipelines/pipelines_table.vue index 466a5b5d635..24d63b99a29 100644 --- a/app/assets/javascripts/commit/pipelines/pipelines_table.vue +++ b/app/assets/javascripts/commit/pipelines/pipelines_table.vue @@ -55,22 +55,20 @@ }, methods: { successCallback(resp) { - return resp.json().then((response) => { - // depending of the endpoint the response can either bring a `pipelines` key or not. - const pipelines = response.pipelines || response; - this.setCommonData(pipelines); + // depending of the endpoint the response can either bring a `pipelines` key or not. + const pipelines = resp.data.pipelines || resp.data; + this.setCommonData(pipelines); - const updatePipelinesEvent = new CustomEvent('update-pipelines-count', { - detail: { - pipelines: response, - }, - }); - - // notifiy to update the count in tabs - if (this.$el.parentElement) { - this.$el.parentElement.dispatchEvent(updatePipelinesEvent); - } + const updatePipelinesEvent = new CustomEvent('update-pipelines-count', { + detail: { + pipelines: resp.data, + }, }); + + // notifiy to update the count in tabs + if (this.$el.parentElement) { + this.$el.parentElement.dispatchEvent(updatePipelinesEvent); + } }, }, }; diff --git a/app/assets/javascripts/pipelines/components/pipelines.vue b/app/assets/javascripts/pipelines/components/pipelines.vue index e0a7284124d..497a09cec65 100644 --- a/app/assets/javascripts/pipelines/components/pipelines.vue +++ b/app/assets/javascripts/pipelines/components/pipelines.vue @@ -7,10 +7,7 @@ import TablePagination from '../../vue_shared/components/table_pagination.vue'; import NavigationTabs from '../../vue_shared/components/navigation_tabs.vue'; import NavigationControls from './nav_controls.vue'; - import { - getParameterByName, - parseQueryStringIntoObject, - } from '../../lib/utils/common_utils'; + import { getParameterByName } from '../../lib/utils/common_utils'; import CIPaginationMixin from '../../vue_shared/mixins/ci_pagination_api_mixin'; export default { @@ -19,10 +16,7 @@ NavigationTabs, NavigationControls, }, - mixins: [ - pipelinesMixin, - CIPaginationMixin, - ], + mixins: [pipelinesMixin, CIPaginationMixin], props: { store: { type: Object, @@ -147,25 +141,26 @@ */ shouldRenderTabs() { const { stateMap } = this.$options; - return this.hasMadeRequest && - [ - stateMap.loading, - stateMap.tableList, - stateMap.error, - stateMap.emptyTab, - ].includes(this.stateToRender); + return ( + this.hasMadeRequest && + [stateMap.loading, stateMap.tableList, stateMap.error, stateMap.emptyTab].includes( + this.stateToRender, + ) + ); }, shouldRenderButtons() { - return (this.newPipelinePath || - this.resetCachePath || - this.ciLintPath) && this.shouldRenderTabs; + return ( + (this.newPipelinePath || this.resetCachePath || this.ciLintPath) && this.shouldRenderTabs + ); }, shouldRenderPagination() { - return !this.isLoading && + return ( + !this.isLoading && this.state.pipelines.length && - this.state.pageInfo.total > this.state.pageInfo.perPage; + this.state.pageInfo.total > this.state.pageInfo.perPage + ); }, emptyTabMessage() { @@ -229,15 +224,13 @@ }, methods: { successCallback(resp) { - return resp.json().then((response) => { - // Because we are polling & the user is interacting verify if the response received - // matches the last request made - if (_.isEqual(parseQueryStringIntoObject(resp.url.split('?')[1]), this.requestData)) { - this.store.storeCount(response.count); - this.store.storePagination(resp.headers); - this.setCommonData(response.pipelines); - } - }); + // Because we are polling & the user is interacting verify if the response received + // matches the last request made + if (_.isEqual(resp.config.params, this.requestData)) { + this.store.storeCount(resp.data.count); + this.store.storePagination(resp.headers); + this.setCommonData(resp.data.pipelines); + } }, /** * Handles URL and query parameter changes. @@ -251,8 +244,9 @@ this.updateInternalState(parameters); // fetch new data - return this.service.getPipelines(this.requestData) - .then((response) => { + return this.service + .getPipelines(this.requestData) + .then(response => { this.isLoading = false; this.successCallback(response); @@ -271,13 +265,11 @@ handleResetRunnersCache(endpoint) { this.isResetCacheButtonLoading = true; - this.service.postAction(endpoint) + this.service + .postAction(endpoint) .then(() => { this.isResetCacheButtonLoading = false; - createFlash( - s__('Pipelines|Project cache successfully reset.'), - 'notice', - ); + createFlash(s__('Pipelines|Project cache successfully reset.'), 'notice'); }) .catch(() => { this.isResetCacheButtonLoading = false; diff --git a/app/assets/javascripts/pipelines/services/pipelines_service.js b/app/assets/javascripts/pipelines/services/pipelines_service.js index 47736fc5f42..001286f5d52 100644 --- a/app/assets/javascripts/pipelines/services/pipelines_service.js +++ b/app/assets/javascripts/pipelines/services/pipelines_service.js @@ -1,35 +1,27 @@ -/* eslint-disable class-methods-use-this */ -import Vue from 'vue'; -import VueResource from 'vue-resource'; -import '../../vue_shared/vue_resource_interceptor'; - -Vue.use(VueResource); +import axios from '../../lib/utils/axios_utils'; export default class PipelinesService { - /** - * Commits and merge request endpoints need to be requested with `.json`. - * - * The url provided to request the pipelines in the new merge request - * page already has `.json`. - * - * @param {String} root - */ + * Commits and merge request endpoints need to be requested with `.json`. + * + * The url provided to request the pipelines in the new merge request + * page already has `.json`. + * + * @param {String} root + */ constructor(root) { - let endpoint; - if (root.indexOf('.json') === -1) { - endpoint = `${root}.json`; + this.endpoint = `${root}.json`; } else { - endpoint = root; + this.endpoint = root; } - - this.pipelines = Vue.resource(endpoint); } getPipelines(data = {}) { const { scope, page } = data; - return this.pipelines.get({ scope, page }); + return axios.get(this.endpoint, { + params: { scope, page }, + }); } /** @@ -38,7 +30,8 @@ export default class PipelinesService { * @param {String} endpoint * @return {Promise} */ + // eslint-disable-next-line class-methods-use-this postAction(endpoint) { - return Vue.http.post(`${endpoint}.json`); + return axios.post(`${endpoint}.json`); } } diff --git a/changelogs/unreleased/40487-axios-pipelines.yml b/changelogs/unreleased/40487-axios-pipelines.yml new file mode 100644 index 00000000000..437d5e87e1a --- /dev/null +++ b/changelogs/unreleased/40487-axios-pipelines.yml @@ -0,0 +1,4 @@ +title: Replace vue resource with axios in pipelines table +merge_request: +author: +type: other \ No newline at end of file diff --git a/spec/javascripts/commit/pipelines/pipelines_spec.js b/spec/javascripts/commit/pipelines/pipelines_spec.js index 0afe09d87bc..53820770f3f 100644 --- a/spec/javascripts/commit/pipelines/pipelines_spec.js +++ b/spec/javascripts/commit/pipelines/pipelines_spec.js @@ -1,113 +1,82 @@ -import _ from 'underscore'; import Vue from 'vue'; +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import pipelinesTable from '~/commit/pipelines/pipelines_table.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; describe('Pipelines table in Commits and Merge requests', () => { const jsonFixtureName = 'pipelines/pipelines.json'; let pipeline; let PipelinesTable; + let mock; + let vm; preloadFixtures(jsonFixtureName); beforeEach(() => { + mock = new MockAdapter(axios); + const pipelines = getJSONFixture(jsonFixtureName).pipelines; PipelinesTable = Vue.extend(pipelinesTable); pipeline = pipelines.find(p => p.user !== null && p.commit !== null); }); + afterEach(() => { + vm.$destroy(); + mock.restore(); + }); + describe('successful request', () => { describe('without pipelines', () => { - const pipelinesEmptyResponse = (request, next) => { - next(request.respondWith(JSON.stringify([]), { - status: 200, - })); - }; - beforeEach(function () { - Vue.http.interceptors.push(pipelinesEmptyResponse); + mock.onGet('endpoint.json').reply(200, []); - this.component = new PipelinesTable({ - propsData: { - endpoint: 'endpoint', - helpPagePath: 'foo', - emptyStateSvgPath: 'foo', - errorStateSvgPath: 'foo', - autoDevopsHelpPath: 'foo', - }, - }).$mount(); - }); - - afterEach(function () { - Vue.http.interceptors = _.without( - Vue.http.interceptors, pipelinesEmptyResponse, - ); - this.component.$destroy(); + vm = mountComponent(PipelinesTable, { + endpoint: 'endpoint.json', + helpPagePath: 'foo', + emptyStateSvgPath: 'foo', + errorStateSvgPath: 'foo', + autoDevopsHelpPath: 'foo', + }); }); it('should render the empty state', function (done) { setTimeout(() => { - expect(this.component.$el.querySelector('.empty-state')).toBeDefined(); - expect(this.component.$el.querySelector('.realtime-loading')).toBe(null); - expect(this.component.$el.querySelector('.js-pipelines-error-state')).toBe(null); + expect(vm.$el.querySelector('.empty-state')).toBeDefined(); + expect(vm.$el.querySelector('.realtime-loading')).toBe(null); + expect(vm.$el.querySelector('.js-pipelines-error-state')).toBe(null); done(); - }, 1); + }, 0); }); }); describe('with pipelines', () => { - const pipelinesResponse = (request, next) => { - next(request.respondWith(JSON.stringify([pipeline]), { - status: 200, - })); - }; - beforeEach(() => { - Vue.http.interceptors.push(pipelinesResponse); - - this.component = new PipelinesTable({ - propsData: { - endpoint: 'endpoint', - helpPagePath: 'foo', - emptyStateSvgPath: 'foo', - errorStateSvgPath: 'foo', - autoDevopsHelpPath: 'foo', - }, - }).$mount(); - }); - - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, pipelinesResponse, - ); - this.component.$destroy(); + mock.onGet('endpoint.json').reply(200, [pipeline]); + vm = mountComponent(PipelinesTable, { + endpoint: 'endpoint.json', + helpPagePath: 'foo', + emptyStateSvgPath: 'foo', + errorStateSvgPath: 'foo', + autoDevopsHelpPath: 'foo', + }); }); it('should render a table with the received pipelines', (done) => { setTimeout(() => { - expect(this.component.$el.querySelectorAll('.ci-table .commit').length).toEqual(1); - expect(this.component.$el.querySelector('.realtime-loading')).toBe(null); - expect(this.component.$el.querySelector('.empty-state')).toBe(null); - expect(this.component.$el.querySelector('.js-pipelines-error-state')).toBe(null); + expect(vm.$el.querySelectorAll('.ci-table .commit').length).toEqual(1); + expect(vm.$el.querySelector('.realtime-loading')).toBe(null); + expect(vm.$el.querySelector('.empty-state')).toBe(null); + expect(vm.$el.querySelector('.js-pipelines-error-state')).toBe(null); done(); }, 0); }); }); describe('pipeline badge counts', () => { - const pipelinesResponse = (request, next) => { - next(request.respondWith(JSON.stringify([pipeline]), { - status: 200, - })); - }; - beforeEach(() => { - Vue.http.interceptors.push(pipelinesResponse); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, pipelinesResponse); - this.component.$destroy(); + mock.onGet('endpoint.json').reply(200, [pipeline]); }); it('should receive update-pipelines-count event', (done) => { @@ -119,54 +88,38 @@ describe('Pipelines table in Commits and Merge requests', () => { done(); }); - this.component = new PipelinesTable({ - propsData: { - endpoint: 'endpoint', - helpPagePath: 'foo', - emptyStateSvgPath: 'foo', - errorStateSvgPath: 'foo', - autoDevopsHelpPath: 'foo', - }, - }).$mount(); - element.appendChild(this.component.$el); + vm = mountComponent(PipelinesTable, { + endpoint: 'endpoint.json', + helpPagePath: 'foo', + emptyStateSvgPath: 'foo', + errorStateSvgPath: 'foo', + autoDevopsHelpPath: 'foo', + }); + + element.appendChild(vm.$el); }); }); }); describe('unsuccessfull request', () => { - const pipelinesErrorResponse = (request, next) => { - next(request.respondWith(JSON.stringify([]), { - status: 500, - })); - }; + beforeEach(() => { + mock.onGet('endpoint.json').reply(500, []); - beforeEach(function () { - Vue.http.interceptors.push(pipelinesErrorResponse); - - this.component = new PipelinesTable({ - propsData: { - endpoint: 'endpoint', - helpPagePath: 'foo', - emptyStateSvgPath: 'foo', - errorStateSvgPath: 'foo', - autoDevopsHelpPath: 'foo', - }, - }).$mount(); - }); - - afterEach(function () { - Vue.http.interceptors = _.without( - Vue.http.interceptors, pipelinesErrorResponse, - ); - this.component.$destroy(); + vm = mountComponent(PipelinesTable, { + endpoint: 'endpoint.json', + helpPagePath: 'foo', + emptyStateSvgPath: 'foo', + errorStateSvgPath: 'foo', + autoDevopsHelpPath: 'foo', + }); }); it('should render error state', function (done) { setTimeout(() => { - expect(this.component.$el.querySelector('.js-pipelines-error-state')).toBeDefined(); - expect(this.component.$el.querySelector('.realtime-loading')).toBe(null); - expect(this.component.$el.querySelector('.js-empty-state')).toBe(null); - expect(this.component.$el.querySelector('.ci-table')).toBe(null); + expect(vm.$el.querySelector('.js-pipelines-error-state')).toBeDefined(); + expect(vm.$el.querySelector('.realtime-loading')).toBe(null); + expect(vm.$el.querySelector('.js-empty-state')).toBe(null); + expect(vm.$el.querySelector('.ci-table')).toBe(null); done(); }, 0); }); diff --git a/spec/javascripts/pipelines/pipelines_spec.js b/spec/javascripts/pipelines/pipelines_spec.js index 7e242eb45e1..d79544f83ad 100644 --- a/spec/javascripts/pipelines/pipelines_spec.js +++ b/spec/javascripts/pipelines/pipelines_spec.js @@ -1,5 +1,6 @@ -import _ from 'underscore'; import Vue from 'vue'; +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import pipelinesComp from '~/pipelines/components/pipelines.vue'; import Store from '~/pipelines/stores/pipelines_store'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; @@ -12,6 +13,8 @@ describe('Pipelines', () => { let PipelinesComponent; let pipelines; let vm; + let mock; + const paths = { endpoint: 'twitter/flight/pipelines.json', autoDevopsPath: '/help/topics/autodevops/index.md', @@ -34,6 +37,8 @@ describe('Pipelines', () => { }; beforeEach(() => { + mock = new MockAdapter(axios); + pipelines = getJSONFixture(jsonFixtureName); PipelinesComponent = Vue.extend(pipelinesComp); @@ -41,38 +46,14 @@ describe('Pipelines', () => { afterEach(() => { vm.$destroy(); + mock.restore(); }); - const pipelinesInterceptor = (request, next) => { - next(request.respondWith(JSON.stringify(pipelines), { - status: 200, - })); - }; - - const emptyStateInterceptor = (request, next) => { - next(request.respondWith(JSON.stringify({ - pipelines: [], - count: { - all: 0, - pending: 0, - running: 0, - finished: 0, - }, - }), { - status: 200, - })); - }; - - const errorInterceptor = (request, next) => { - next(request.respondWith(JSON.stringify({}), { - status: 500, - })); - }; - describe('With permission', () => { describe('With pipelines in main tab', () => { beforeEach((done) => { - Vue.http.interceptors.push(pipelinesInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(200, pipelines); + vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: true, @@ -85,12 +66,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, pipelinesInterceptor, - ); - }); - it('renders tabs', () => { expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All'); }); @@ -116,7 +91,15 @@ describe('Pipelines', () => { describe('Without pipelines on main tab with CI', () => { beforeEach((done) => { - Vue.http.interceptors.push(emptyStateInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(200, { + pipelines: [], + count: { + all: 0, + pending: 0, + running: 0, + finished: 0, + }, + }); vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: true, @@ -129,12 +112,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, emptyStateInterceptor, - ); - }); - it('renders tabs', () => { expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All'); }); @@ -158,7 +135,15 @@ describe('Pipelines', () => { describe('Without pipelines nor CI', () => { beforeEach((done) => { - Vue.http.interceptors.push(emptyStateInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(200, { + pipelines: [], + count: { + all: 0, + pending: 0, + running: 0, + finished: 0, + }, + }); vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: false, @@ -171,12 +156,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, emptyStateInterceptor, - ); - }); - it('renders empty state', () => { expect(vm.$el.querySelector('.js-empty-state h4').textContent.trim()).toEqual('Build with confidence'); expect(vm.$el.querySelector('.js-get-started-pipelines').getAttribute('href')).toEqual(paths.helpPagePath); @@ -192,7 +171,7 @@ describe('Pipelines', () => { describe('When API returns error', () => { beforeEach((done) => { - Vue.http.interceptors.push(errorInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(500, {}); vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: false, @@ -205,12 +184,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, errorInterceptor, - ); - }); - it('renders tabs', () => { expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All'); }); @@ -230,7 +203,8 @@ describe('Pipelines', () => { describe('Without permission', () => { describe('With pipelines in main tab', () => { beforeEach((done) => { - Vue.http.interceptors.push(pipelinesInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(200, pipelines); + vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: false, @@ -243,12 +217,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, pipelinesInterceptor, - ); - }); - it('renders tabs', () => { expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All'); }); @@ -268,7 +236,16 @@ describe('Pipelines', () => { describe('Without pipelines on main tab with CI', () => { beforeEach((done) => { - Vue.http.interceptors.push(emptyStateInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(200, { + pipelines: [], + count: { + all: 0, + pending: 0, + running: 0, + finished: 0, + }, + }); + vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: true, @@ -281,11 +258,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, emptyStateInterceptor, - ); - }); it('renders tabs', () => { expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All'); }); @@ -303,7 +275,16 @@ describe('Pipelines', () => { describe('Without pipelines nor CI', () => { beforeEach((done) => { - Vue.http.interceptors.push(emptyStateInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(200, { + pipelines: [], + count: { + all: 0, + pending: 0, + running: 0, + finished: 0, + }, + }); + vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: false, @@ -316,12 +297,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, emptyStateInterceptor, - ); - }); - it('renders empty state without button to set CI', () => { expect(vm.$el.querySelector('.js-empty-state').textContent.trim()).toEqual('This project is not currently set up to run pipelines.'); expect(vm.$el.querySelector('.js-get-started-pipelines')).toBeNull(); @@ -337,7 +312,8 @@ describe('Pipelines', () => { describe('When API returns error', () => { beforeEach((done) => { - Vue.http.interceptors.push(errorInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(500, {}); + vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: false, @@ -350,12 +326,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, errorInterceptor, - ); - }); - it('renders tabs', () => { expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All'); }); @@ -375,7 +345,8 @@ describe('Pipelines', () => { describe('successfull request', () => { describe('with pipelines', () => { beforeEach(() => { - Vue.http.interceptors.push(pipelinesInterceptor); + mock.onGet('twitter/flight/pipelines.json').reply(200, pipelines); + vm = mountComponent(PipelinesComponent, { store: new Store(), hasGitlabCi: true, @@ -384,12 +355,6 @@ describe('Pipelines', () => { }); }); - afterEach(() => { - Vue.http.interceptors = _.without( - Vue.http.interceptors, pipelinesInterceptor, - ); - }); - it('should render table', (done) => { setTimeout(() => { expect(vm.$el.querySelector('.table-holder')).toBeDefined();