From b5b304466fbd8904196afeaa65adeb5282b21987 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 10:34:12 +0000 Subject: [PATCH 1/8] Converted mini_pipeline_graph_dropdown.js to axios --- .../mini_pipeline_graph_dropdown.js | 26 +++--- .../mini_pipeline_graph_dropdown_spec.js | 79 ++++++++++++------- 2 files changed, 61 insertions(+), 44 deletions(-) diff --git a/app/assets/javascripts/mini_pipeline_graph_dropdown.js b/app/assets/javascripts/mini_pipeline_graph_dropdown.js index ca3d271663b..c7bccd483ac 100644 --- a/app/assets/javascripts/mini_pipeline_graph_dropdown.js +++ b/app/assets/javascripts/mini_pipeline_graph_dropdown.js @@ -1,5 +1,6 @@ /* eslint-disable no-new */ -import Flash from './flash'; +import flash from './flash'; +import axios from './lib/utils/axios_utils'; /** * In each pipelines table we have a mini pipeline graph for each pipeline. @@ -78,27 +79,22 @@ export default class MiniPipelineGraph { const button = e.relatedTarget; const endpoint = button.dataset.stageEndpoint; - return $.ajax({ - dataType: 'json', - type: 'GET', - url: endpoint, - beforeSend: () => { - this.renderBuildsList(button, ''); - this.toggleLoading(button); - }, - success: (data) => { + this.renderBuildsList(button, ''); + this.toggleLoading(button); + + axios.get(endpoint) + .then(({ data }) => { this.toggleLoading(button); this.renderBuildsList(button, data.html); this.stopDropdownClickPropagation(); - }, - error: () => { + }) + .catch(() => { this.toggleLoading(button); if ($(button).parent().hasClass('open')) { $(button).dropdown('toggle'); } - new Flash('An error occurred while fetching the builds.', 'alert'); - }, - }); + flash('An error occurred while fetching the builds.', 'alert'); + }); } /** diff --git a/spec/javascripts/mini_pipeline_graph_dropdown_spec.js b/spec/javascripts/mini_pipeline_graph_dropdown_spec.js index 481b46c3ac6..6fa6f44f953 100644 --- a/spec/javascripts/mini_pipeline_graph_dropdown_spec.js +++ b/spec/javascripts/mini_pipeline_graph_dropdown_spec.js @@ -1,7 +1,9 @@ /* eslint-disable no-new */ +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import MiniPipelineGraph from '~/mini_pipeline_graph_dropdown'; -import '~/flash'; +import timeoutPromise from './helpers/set_timeout_promise_helper'; describe('Mini Pipeline Graph Dropdown', () => { preloadFixtures('static/mini_dropdown_graph.html.raw'); @@ -27,6 +29,16 @@ describe('Mini Pipeline Graph Dropdown', () => { }); describe('When dropdown is clicked', () => { + let mock; + + beforeEach(() => { + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + }); + it('should call getBuildsList', () => { const getBuildsListSpy = spyOn( MiniPipelineGraph.prototype, @@ -41,46 +53,55 @@ describe('Mini Pipeline Graph Dropdown', () => { }); it('should make a request to the endpoint provided in the html', () => { - const ajaxSpy = spyOn($, 'ajax').and.callFake(function () {}); + const ajaxSpy = spyOn(axios, 'get').and.callThrough(); + + mock.onGet('foobar').reply(200, { + html: '', + }); new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); document.querySelector('.js-builds-dropdown-button').click(); - expect(ajaxSpy.calls.allArgs()[0][0].url).toEqual('foobar'); + expect(ajaxSpy.calls.allArgs()[0][0]).toEqual('foobar'); }); - it('should not close when user uses cmd/ctrl + click', () => { - spyOn($, 'ajax').and.callFake(function (params) { - params.success({ - html: `
  • - - - build - - -
    • `, - }); + it('should not close when user uses cmd/ctrl + click', (done) => { + mock.onGet('foobar').reply(200, { + html: `
  • + + + build + + +
    • `, }); new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); document.querySelector('.js-builds-dropdown-button').click(); - document.querySelector('a.mini-pipeline-graph-dropdown-item').click(); + timeoutPromise() + .then(() => { + document.querySelector('a.mini-pipeline-graph-dropdown-item').click(); + }) + .then(timeoutPromise) + .then(() => { + expect($('.js-builds-dropdown-list').is(':visible')).toEqual(true); + }) + .then(done) + .catch(done.fail); + }); - expect($('.js-builds-dropdown-list').is(':visible')).toEqual(true); + it('should close the dropdown when request returns an error', (done) => { + mock.onGet('foobar').networkError(); + + new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); + + document.querySelector('.js-builds-dropdown-button').click(); + + setTimeout(() => { + expect($('.js-builds-dropdown-tests .dropdown').hasClass('open')).toEqual(false); + done(); + }); }); }); - - it('should close the dropdown when request returns an error', (done) => { - spyOn($, 'ajax').and.callFake(options => options.error()); - - new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); - - document.querySelector('.js-builds-dropdown-button').click(); - - setTimeout(() => { - expect($('.js-builds-dropdown-tests .dropdown').hasClass('open')).toEqual(false); - done(); - }, 0); - }); }); From c09a89c7557a93b728bab9eef9175abb1fd458d4 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 10:42:49 +0000 Subject: [PATCH 2/8] Converted branch_graph.js to axios --- app/assets/javascripts/network/branch_graph.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/network/branch_graph.js b/app/assets/javascripts/network/branch_graph.js index 5aad3908eb6..d3edcb724f1 100644 --- a/app/assets/javascripts/network/branch_graph.js +++ b/app/assets/javascripts/network/branch_graph.js @@ -1,5 +1,8 @@ /* eslint-disable func-names, space-before-function-paren, no-var, wrap-iife, quotes, comma-dangle, one-var, one-var-declaration-per-line, no-mixed-operators, no-loop-func, no-floating-decimal, consistent-return, no-unused-vars, prefer-template, prefer-arrow-callback, camelcase, max-len */ +import { __ } from '../locale'; +import axios from '../lib/utils/axios_utils'; +import flash from '../flash'; import Raphael from './raphael'; export default (function() { @@ -26,16 +29,13 @@ export default (function() { } BranchGraph.prototype.load = function() { - return $.ajax({ - url: this.options.url, - method: "get", - dataType: "json", - success: $.proxy(function(data) { + axios.get(this.options.url) + .then(({ data }) => { $(".loading", this.element).hide(); this.prepareData(data.days, data.commits); - return this.buildGraph(); - }, this) - }); + this.buildGraph(); + }) + .catch(() => __('Error fetching network graph.')); }; BranchGraph.prototype.prepareData = function(days, commits) { From 1c8553f21e1cbfa730f47ea8c0be4080a9238f89 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 10:50:18 +0000 Subject: [PATCH 3/8] Converted notes.js to axios --- app/assets/javascripts/notes.js | 35 ++++++++++++++------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index a2b8e6f6495..bcb342f407f 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -16,6 +16,7 @@ import Autosize from 'autosize'; import 'vendor/jquery.caret'; // required by jquery.atwho import 'vendor/jquery.atwho'; import AjaxCache from '~/lib/utils/ajax_cache'; +import axios from './lib/utils/axios_utils'; import { getLocationHash } from './lib/utils/url_utility'; import Flash from './flash'; import CommentTypeToggle from './comment_type_toggle'; @@ -252,26 +253,20 @@ export default class Notes { return; } this.refreshing = true; - return $.ajax({ - url: this.notes_url, - headers: { 'X-Last-Fetched-At': this.last_fetched_at }, - dataType: 'json', - success: (function(_this) { - return function(data) { - var notes; - notes = data.notes; - _this.last_fetched_at = data.last_fetched_at; - _this.setPollingInterval(data.notes.length); - return $.each(notes, function(i, note) { - _this.renderNote(note); - }); - }; - })(this) - }).always((function(_this) { - return function() { - return _this.refreshing = false; - }; - })(this)); + axios.get(this.notes_url, { + headers: { + 'X-Last-Fetched-At': this.last_fetched_at, + }, + }).then(({ data }) => { + const notes = data.notes; + this.last_fetched_at = data.last_fetched_at; + this.setPollingInterval(data.notes.length); + $.each(notes, (i, note) => this.renderNote(note)); + + this.refreshing = false; + }).catch(() => { + this.refreshing = false; + }); } /** From ab8e3a5595a441eb5b24cd7db5a877b65dcef704 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 10:53:36 +0000 Subject: [PATCH 4/8] Converted notifications_form.js to axios --- app/assets/javascripts/notifications_form.js | 38 ++++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/notifications_form.js b/app/assets/javascripts/notifications_form.js index 4534360d577..4e0afe13590 100644 --- a/app/assets/javascripts/notifications_form.js +++ b/app/assets/javascripts/notifications_form.js @@ -1,3 +1,7 @@ +import { __ } from './locale'; +import axios from './lib/utils/axios_utils'; +import flash from './flash'; + export default class NotificationsForm { constructor() { this.toggleCheckbox = this.toggleCheckbox.bind(this); @@ -27,24 +31,20 @@ export default class NotificationsForm { saveEvent($checkbox, $parent) { const form = $parent.parents('form:first'); - return $.ajax({ - url: form.attr('action'), - method: form.attr('method'), - dataType: 'json', - data: form.serialize(), - beforeSend: () => { - this.showCheckboxLoadingSpinner($parent); - }, - }).done((data) => { - $checkbox.enable(); - if (data.saved) { - $parent.find('.custom-notification-event-loading').toggleClass('fa-spin fa-spinner fa-check is-done'); - setTimeout(() => { - $parent.removeClass('is-loading') - .find('.custom-notification-event-loading') - .toggleClass('fa-spin fa-spinner fa-check is-done'); - }, 2000); - } - }); + this.showCheckboxLoadingSpinner($parent); + + axios[form.attr('method')](form.attr('action'), form.serialize()) + .then(({ data }) => { + $checkbox.enable(); + if (data.saved) { + $parent.find('.custom-notification-event-loading').toggleClass('fa-spin fa-spinner fa-check is-done'); + setTimeout(() => { + $parent.removeClass('is-loading') + .find('.custom-notification-event-loading') + .toggleClass('fa-spin fa-spinner fa-check is-done'); + }, 2000); + } + }) + .catch(() => flash(__('There was an error saving your notification settings.'))); } } From ee1c471bad95cb640ea63393954825dd5a68a9e2 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 11:12:37 +0000 Subject: [PATCH 5/8] Converted pager.js to axios --- app/assets/javascripts/pager.js | 33 +++++++------- spec/javascripts/pager_spec.js | 76 ++++++++++++++++++++++++++------- 2 files changed, 77 insertions(+), 32 deletions(-) diff --git a/app/assets/javascripts/pager.js b/app/assets/javascripts/pager.js index 6552a88b606..fd3105b1960 100644 --- a/app/assets/javascripts/pager.js +++ b/app/assets/javascripts/pager.js @@ -1,4 +1,5 @@ import { getParameterByName } from '~/lib/utils/common_utils'; +import axios from './lib/utils/axios_utils'; import { removeParams } from './lib/utils/url_utility'; const ENDLESS_SCROLL_BOTTOM_PX = 400; @@ -22,24 +23,22 @@ export default { getOld() { this.loading.show(); - $.ajax({ - type: 'GET', - url: this.url, - data: `limit=${this.limit}&offset=${this.offset}`, - dataType: 'json', - error: () => this.loading.hide(), - success: (data) => { - this.append(data.count, this.prepareData(data.html)); - this.callback(); - - // keep loading until we've filled the viewport height - if (!this.disable && !this.isScrollable()) { - this.getOld(); - } else { - this.loading.hide(); - } + axios.get(this.url, { + params: { + limit: this.limit, + offset: this.offset, }, - }); + }).then(({ data }) => { + this.append(data.count, this.prepareData(data.html)); + this.callback(); + + // keep loading until we've filled the viewport height + if (!this.disable && !this.isScrollable()) { + this.getOld(); + } else { + this.loading.hide(); + } + }).catch(() => this.loading.hide()); }, append(count, html) { diff --git a/spec/javascripts/pager_spec.js b/spec/javascripts/pager_spec.js index 2fd87754238..fd9b83e3514 100644 --- a/spec/javascripts/pager_spec.js +++ b/spec/javascripts/pager_spec.js @@ -1,5 +1,6 @@ /* global fixture */ - +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import * as utils from '~/lib/utils/url_utility'; import Pager from '~/pager'; @@ -9,7 +10,6 @@ describe('pager', () => { beforeEach(() => { setFixtures('
    '); - spyOn($, 'ajax'); }); afterEach(() => { @@ -47,39 +47,85 @@ describe('pager', () => { }); describe('getOld', () => { + const urlRegex = /\/some_list(.*)$/; + let mock; + + function mockSuccess() { + mock.onGet(urlRegex).reply(200, { + count: 20, + html: '', + }); + } + + function mockError() { + mock.onGet(urlRegex).networkError(); + } + beforeEach(() => { setFixtures('
    '); + spyOn(axios, 'get').and.callThrough(); + Pager.init(); + + mock = new MockAdapter(axios); }); - it('shows loader while loading next page', () => { + afterEach(() => { + mock.restore(); + }); + + it('shows loader while loading next page', (done) => { + mockSuccess(); + spyOn(Pager.loading, 'show'); Pager.getOld(); - expect(Pager.loading.show).toHaveBeenCalled(); + + setTimeout(() => { + expect(Pager.loading.show).toHaveBeenCalled(); + + done(); + }); }); - it('hides loader on success', () => { - spyOn($, 'ajax').and.callFake(options => options.success({})); + it('hides loader on success', (done) => { + mockSuccess(); + spyOn(Pager.loading, 'hide'); Pager.getOld(); - expect(Pager.loading.hide).toHaveBeenCalled(); + + setTimeout(() => { + expect(Pager.loading.hide).toHaveBeenCalled(); + + done(); + }); }); - it('hides loader on error', () => { - spyOn($, 'ajax').and.callFake(options => options.error()); + it('hides loader on error', (done) => { + mockError(); + spyOn(Pager.loading, 'hide'); Pager.getOld(); - expect(Pager.loading.hide).toHaveBeenCalled(); + + setTimeout(() => { + expect(Pager.loading.hide).toHaveBeenCalled(); + + done(); + }); }); - it('sends request to url with offset and limit params', () => { - spyOn($, 'ajax'); + it('sends request to url with offset and limit params', (done) => { Pager.offset = 100; Pager.limit = 20; Pager.getOld(); - const [{ data, url }] = $.ajax.calls.argsFor(0); - expect(data).toBe('limit=20&offset=100'); - expect(url).toBe('/some_list'); + + setTimeout(() => { + const [url, params] = $.ajax.calls.argsFor(0); + console.log(url, params); + // expect(data).toBe('limit=20&offset=100'); + // expect(url).toBe('/some_list'); + + done(); + }); }); }); }); From 43f1088f5b086b95a3d5cdd90a33d26bb483cba5 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 11:18:01 +0000 Subject: [PATCH 6/8] Converted usage_ping.js to use axios also removed dependancy on jQuery because it is super simple & not required --- .../pages/admin/cohorts/usage_ping.js | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/pages/admin/cohorts/usage_ping.js b/app/assets/javascripts/pages/admin/cohorts/usage_ping.js index 2389056bd02..914a9661c27 100644 --- a/app/assets/javascripts/pages/admin/cohorts/usage_ping.js +++ b/app/assets/javascripts/pages/admin/cohorts/usage_ping.js @@ -1,12 +1,13 @@ -export default function UsagePing() { - const usageDataUrl = $('.usage-data').data('endpoint'); +import axios from '../../../lib/utils/axios_utils'; +import { __ } from '../../../locale'; +import flash from '../../../flash'; - $.ajax({ - type: 'GET', - url: usageDataUrl, - dataType: 'html', - success(html) { - $('.usage-data').html(html); - }, - }); +export default function UsagePing() { + const el = document.querySelector('.usage-data'); + + axios.get(el.dataset.endpoint, { + responseType: 'text', + }).then(({ data }) => { + el.innerHTML = data; + }).catch(() => flash(__('Error fetching usage ping data.'))); } From 4cea24f89c757dafcba190b46d1f5f866acd50f6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 11:32:31 +0000 Subject: [PATCH 7/8] Converted todos.js to axios --- .../pages/dashboard/todos/index/todos.js | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/pages/dashboard/todos/index/todos.js b/app/assets/javascripts/pages/dashboard/todos/index/todos.js index e976a3d2f1d..b3f6a72fdcb 100644 --- a/app/assets/javascripts/pages/dashboard/todos/index/todos.js +++ b/app/assets/javascripts/pages/dashboard/todos/index/todos.js @@ -2,6 +2,9 @@ import { visitUrl } from '~/lib/utils/url_utility'; import UsersSelect from '~/users_select'; import { isMetaClick } from '~/lib/utils/common_utils'; +import { __ } from '../../../../locale'; +import flash from '../../../../flash'; +import axios from '../../../../lib/utils/axios_utils'; export default class Todos { constructor() { @@ -59,18 +62,12 @@ export default class Todos { const target = e.target; target.setAttribute('disabled', true); target.classList.add('disabled'); - $.ajax({ - type: 'POST', - url: target.dataset.href, - dataType: 'json', - data: { - '_method': target.dataset.method, - }, - success: (data) => { + + axios[target.dataset.method](target.dataset.href) + .then(({ data }) => { this.updateRowState(target); - return this.updateBadges(data); - }, - }); + this.updateBadges(data); + }).catch(() => flash(__('Error updating todo status.'))); } updateRowState(target) { @@ -98,19 +95,15 @@ export default class Todos { e.preventDefault(); const target = e.currentTarget; - const requestData = { '_method': target.dataset.method, ids: this.todo_ids }; target.setAttribute('disabled', true); target.classList.add('disabled'); - $.ajax({ - type: 'POST', - url: target.dataset.href, - dataType: 'json', - data: requestData, - success: (data) => { - this.updateAllState(target, data); - return this.updateBadges(data); - }, - }); + + axios[target.dataset.method](target.dataset.href, { + ids: this.todo_ids, + }).then(({ data }) => { + this.updateAllState(target, data); + this.updateBadges(data); + }).catch(() => flash(__('Error updating status for all todos.'))); } updateAllState(target, data) { From fe2e8dba22471eb39a703672c5d8039c20e94fa6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 12:26:55 +0000 Subject: [PATCH 8/8] fixed infinite loop crashing tests --- spec/javascripts/pager_spec.js | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/spec/javascripts/pager_spec.js b/spec/javascripts/pager_spec.js index fd9b83e3514..b09494f0b77 100644 --- a/spec/javascripts/pager_spec.js +++ b/spec/javascripts/pager_spec.js @@ -47,12 +47,12 @@ describe('pager', () => { }); describe('getOld', () => { - const urlRegex = /\/some_list(.*)$/; + const urlRegex = /(.*)some_list(.*)$/; let mock; function mockSuccess() { mock.onGet(urlRegex).reply(200, { - count: 20, + count: 0, html: '', }); } @@ -65,9 +65,9 @@ describe('pager', () => { setFixtures('
    '); spyOn(axios, 'get').and.callThrough(); - Pager.init(); - mock = new MockAdapter(axios); + + Pager.init(); }); afterEach(() => { @@ -119,10 +119,15 @@ describe('pager', () => { Pager.getOld(); setTimeout(() => { - const [url, params] = $.ajax.calls.argsFor(0); - console.log(url, params); - // expect(data).toBe('limit=20&offset=100'); - // expect(url).toBe('/some_list'); + const [url, params] = axios.get.calls.argsFor(0); + + expect(params).toEqual({ + params: { + limit: 20, + offset: 100, + }, + }); + expect(url).toBe('/some_list'); done(); });